-
Notifications
You must be signed in to change notification settings - Fork 13.2k
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
Allow int literals for pattern types with int base types #137715
base: master
Are you sure you want to change the base?
Conversation
changes to the core type system HIR ty lowering was modified cc @fmease changes to the core type system Some changes occurred in src/tools/clippy cc @rust-lang/clippy |
2845be6
to
897ace8
Compare
This comment was marked as resolved.
This comment was marked as resolved.
e5f8770
to
e9f5d48
Compare
if layout.uninhabited { | ||
None | ||
} else { |
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.
Patterns types of integers cant be uninhabited can they?
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.
even if they were why would we want to fallback to an integer var instead of using the expected (uninhabited) pattern type
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 don't know if u32 is 10..2
exists, but if it does then it's uninhabited.
(And it might be nice to just allow the type to exist so that we don't need where {MIN <= MAX}
bounds on things.)
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.
This PR makes us error when encountering wrapping ranges like 10..2 so that wouldn't be uninhabited as of this PR. Though if it did exist I could imagine it being uninhabited
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.
oops yea that was just a leftover from doing various pattern types things in parallel. I turned it into an assert
fn negative_lit_in_range() -> pattern_type!(i8 is -5..5) { | ||
-2 | ||
//~^ ERROR: cannot apply unary operator `-` to type `(i8) is -5..=4` | ||
} |
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.
Is this something you expect to start supporting with your other work about having negated literals explicit in the ast/hir?
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.
Yes, that's why I started that work
e9f5d48
to
f87e58f
Compare
r? @BoxyUwU
I also added an error at layout computation time for layouts that contain wrapping ranges (happens at monomorphization time). This is obviously hacky, but at least prevents such types from making it to codegen for now. It made writing the tests for int literals easier as I didn't have to think about that edge case
Basically this PR allows you to stop using transmutes for creating pattern types and instead just use literals:
works, and if the literal is out of range you get a type mismatch because it just stays at the base type and the base type can't be coerced to the pattern type.
cc @joshtriplett @scottmcm