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

Document overflow behavior, add tests. #130

Merged
merged 2 commits into from
Jun 25, 2020
Merged

Conversation

JimLarson
Copy link
Contributor

No description provided.

@JimLarson
Copy link
Contributor Author

Closes #131

@@ -827,6 +827,12 @@ values. The ordering operators obey the usual algebraic properties, i.e. `e1 <=
e2` gives the same result as `!(e1 > e2)` as well as `(e1 < e2) || (e1 == e2)`
when the expressions involved do not have side effects.

### Overflow
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could we add a test to ensure negating the minimum int value will overflow?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. Thanks, good catch. In cel-go it wraps back to itself, but for consistency we should expect an error.

@JimLarson
Copy link
Contributor Author

PTAL

}
test {
name: "int64_min_negate"
description: "Negated LLONG_MIN is not representable."
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: extra L in LONG?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think Gabriel was using standard C constants, where "long" might only be 32-bits, so it's for "long long min", guaranteeing at least 64 bits. Long long could be bigger than that, but C's flexibility for integer sizes is mostly theoretical these days - at least outside the embedded market.

@JimLarson JimLarson merged commit e34c5ef into google:master Jun 25, 2020
@JimLarson JimLarson deleted the overload branch June 25, 2020 00:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants