Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

config/lang: support math on variables through implicits #1521

Merged
merged 1 commit into from
Apr 14, 2015

Conversation

mitchellh
Copy link
Contributor

Fixes #1381

We always meant for conversion to the correct numeric type to be implicit. There is an edge case though if the first operand to math ops is not a primitive: the type inference doesn't work. This fixes that issue.

// Determine the resulting type we want. We do this by going over
// every expression until we find one with a type we recognize.
// We do this because the first expr might be a string ("var.foo")
// and we need to know what to implicit to.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Implicit" as a verb? I like it!

// We found the type, so leave
if exit {
break
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For a minute I was like "what about the error case?" but now I see it.

If we don't hit anything in the range we like - the initial mathFunc and mathType will implicit everything for int math below. 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep yep!

@phinze
Copy link
Contributor

phinze commented Apr 14, 2015

Good good good. 👌

mitchellh added a commit that referenced this pull request Apr 14, 2015
config/lang: support math on variables through implicits
@mitchellh mitchellh merged commit 2913b93 into master Apr 14, 2015
@mitchellh mitchellh deleted the b-math-vars branch April 14, 2015 16:14
@ghost
Copy link

ghost commented May 3, 2020

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.

@ghost ghost locked and limited conversation to collaborators May 3, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants