Arithmetic operation returns an invalid result

[achille-roussel]

Terraform Version

Terraform v0.8.0

Affected Resource(s)

N/A

Terraform Configuration Files

N/A

Debug Output

https://gist.github.com/achille-roussel/fa9b4e87e5151ccf1a604ab3e3b87f9f

Panic Output

N/A

Expected Behavior

100 * 0.5 should be evaluated to 50

Actual Behavior

100 * 0.5 is evaluated to 0

Steps to Reproduce

1. terraform console
2. 100 * 0.5

Important Factoids

N/A

References

N/A

[stack72]

@mitchellh

% terraform console
> 100 * 0.5
0
> 100 * 2
200
> 100 * 3
300

[apparentlymart]

Hi @achille-roussel. Sorry for this unexpected behavior...

What happened here is that Terraform saw that 100 is an integer and so it converted the 0.5 to an integer, rounding it to zero. You can work around this for the moment by writing either 100.0 * 0.5 or 0.5 * 100 so that Terraform understands that you want it to do floating point arithmetic.

However I do think we should fix this behavior by adding a special case to say that when given an int and a float HIL should prefer to convert the int to float rather than the other way around, regardless of the operand order, to avoid this sort of confusion.

[mitchellh]

@apparentlymart I agree, I tested this behavior in C, Go, and Ruby and it looks like float just has a higher conversion preference.

[apparentlymart]

I made hashicorp/hil#43 to track this, since the fix here will be entirely in HIL. But will leave this one open to represent the need to update the vendored version of HIL in Terraform once the fix is applied.

[mengesb]

Perhaps there's room here to do type casting interpolation as well? float() to force an integer/float input to return a float for example?

The problem with forcing '#.#inputs is that even addition resulting in a.0` float returns an integer:

> "${2.0 + 3.0}"
5

[mengesb]

For even/odd division in the situation of replacing ceil() I've had to resort to the following interpolation:

cidr_block = "${cidrsubnet(var.vpc["cidr"],(var.subnets["public"] + var.subnets["private"] + ((var.subnets["public"] + var.subnets["private"]) % 2))/2, count.index)}"

As part of my plan:

variable "subnets" {
  type              = "map"
  description       = "Public/Private subnet counts"
  default           = {
    public          = 1
    private         = 4
  }
}
resource "aws_subnet" "private-subnets" {
  count                = "${var.subnets["private"]}"
  vpc_id               = "${aws_vpc.chef-vpc.id}"
  availability_zone    = "${data.aws_availability_zones.az.names[(count.index + var.subnets["public"]) % length(data.aws_availability_zones.az.names)]}"
  cidr_block           = "${cidrsubnet(var.vpc["cidr"],(var.subnets["public"] + var.subnets["private"] + ((var.subnets["public"] + var.subnets["private"]) % 2))/2, count.index + var.subnets["public"])}"
  map_public_ip_on_launch = "false"
  tags {
    Name               = "rivate Subnet ${count.index + 1}"
    Terraform          = "${var.vpc["tags_terraform"]}"
    map-public-ip      = "false"
  }
}

[apparentlymart]

@mengesb that example of testing with the console might be a bit misleading since everything always gets converted to string before displaying it, and the conversion from float to string doesn't include the .0 suffix when the float is a whole number. Based on what I know about HIL I believe it is actually returning a float, even though it's indistinguishable from a stringified integer.

Honestly it feels a bit weird to me that HIL even distinguishes ints and floats rather than just having a single double-precision float number type. Perhaps in the long run we should just squash these two types together and just represent integers as whole number floats.

The main justifications for a separate int type in programming languages tend to be performance (faster to do int arithmetic than float arithmetic) and range (on a 64-bit machine the maximum integer is bigger than the maximum float) but neither of those seems compelling for the primary use-cases HIL deals with. Some possible edge-cases to consider are in integrations with external APIs that accept 64-bit integers, where we could find ourselves unable to represent a value from the external system; that's already true on 32-bit builds though, and it doesn't seem to be hurting that much.

I do think we should just do the adjustment of the type conversion rules first to address this concern quickly, but longer term we can think more holistically about the tradeoffs in HIL's type system.

[mengesb]

@apparentlymart there's also

> "${2 + 3.1}" #WRONG -> 5.1
5
> "${ceil((2 + 3.0)/2.0)}" #WRONG -> 3
2
> "${max(5/2.0)}" #WRONG -> 2.5
2
> "${max(5.0/2)}" #CORRECT
2.5
> "${ceil(2.5)}" #CORRECT
3

I'm sure the list goes on.

[apparentlymart]

Right... the rule for the moment is that if you want to do floating point arithmetic you need to make sure the first non-string operand is a float. I plan to address this in a lightweight way in that HIL issue. Longer term we might want to do something more drastic here, but I presume that would have to wait until 0.9 so we can properly manage any compatibility risks that implies.

When we were discussing this on Glitter I came up with this gross hack to force an integer variable to be a float, as a partial workaround for now:

"${var.whatever}.0" / 2.0

...but this is too gross to live so I would not suggest anyone do it unless they are really desperate. It also, unfortunately, does not fully generalize because an arithmetic operation with two string operands will currently convert the strings to integers rather than floats, so at least one of the operands must be actually of type float. 😖

[mengesb]

Pretty sure I tried that ugly hack within interpolation (before I went the route of using modulo to get my odd/even addition modifier) and it didn't work with a string type conversion error.

[ghost]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.