fitzgen opened PR #5673 from shift-right-left-to-mask
to main
:
Thanks to Souper for pointing this one out! cc @regehr (let me know if you aren't interested in being cc'd on these!)
<!--
Please ensure that the following steps are all taken care of before submitting
the PR.
[ ] This has been discussed in issue #..., or if not, please tell us why
here.[ ] A short description of what this does, why it is needed; if the
description becomes long, the matter should probably be discussed in an issue
first.[ ] This PR contains test cases, if meaningful.
- [ ] A reviewer from the core maintainer team has been assigned for this PR.
If you don't know who could review this, please indicate so. The list of
suggested reviewers on the right can help you.Please ensure all communication adheres to the code of conduct.
-->
fitzgen requested cfallin for a review on PR #5673.
cfallin submitted PR review.
cfallin submitted PR review.
cfallin created PR review comment:
Should this be
0xFFFFFFFFFFFFFFFF
instead (i.e.,u64::MAX
)? Otherwise, a comment whyu32::MAX
is sufficient?(Likewise below)
fitzgen submitted PR review.
fitzgen created PR review comment:
D'oh!
fitzgen submitted PR review.
fitzgen created PR review comment:
FWIW: I originally had
-1
but ISLE turns that into ani128
literal in the rust source code and then it fails to type check.
fitzgen updated PR #5673 from shift-right-left-to-mask
to main
.
fitzgen has enabled auto merge for PR #5673.
cfallin submitted PR review.
fitzgen has disabled auto merge for PR #5673.
fitzgen updated PR #5673 from shift-right-left-to-mask
to main
.
fitzgen has enabled auto merge for PR #5673.
fitzgen merged PR #5673.
jameysharp submitted PR review.
jameysharp created PR review comment:
Yup, that's my fault. See #5423 for why I think it's better than the alternative, and #5431 for Chris' proposal for improving this situation in the future.
Last updated: Nov 22 2024 at 16:03 UTC