-
-
Notifications
You must be signed in to change notification settings - Fork 812
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
feat[lang]: allow hex literals for unsigned integer types #4387
base: master
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #4387 +/- ##
==========================================
- Coverage 91.31% 88.61% -2.71%
==========================================
Files 113 113
Lines 16061 16078 +17
Branches 2703 2708 +5
==========================================
- Hits 14666 14247 -419
- Misses 964 1314 +350
- Partials 431 517 +86 ☔ View full report in Codecov by Sentry. |
@@ -2363,7 +2363,16 @@ def infer_kwarg_types(self, node): | |||
for kwarg in node.keywords: | |||
kwarg_name = kwarg.arg | |||
validate_expected_type(kwarg.value, self._kwargs[kwarg_name].typ) | |||
ret[kwarg_name] = get_exact_type_from_node(kwarg.value) | |||
if kwarg_name == "method_id": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe out-of-scope but it would be much better for validate_expected_type
to return the type it inferred
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I/we have tried this a few times before without completing it 😄
@@ -171,6 +171,13 @@ def hello() : | |||
""", | |||
ImmutableViolation, | |||
), | |||
( | |||
# hex values not allowed for signed integer types |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
might be ok to allow this actually
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hm if we allow hex literals for unsigned and signed ints, should we disallow folding involving any hex integer? since we do not know at the time of folding if a hex value should be interpreted as a signed integer or an unsigned integer
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but I think excluding hex literals during folding would not work either because it would break things downstream where we expect a literal value during typechecking
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what if we make the syntax more explicit like native hex bytes? e,g, unsigned hex integers would be u"0xff"
and signed hex integers would be i"0xff"
.
What I did
Resolves #4344.
How I did it
Add Hex nodes as valid literals for integers.
How to verify it
See tests.
Commit message
Description for the changelog
Cute Animal Picture