-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Pattern types MVP #107606
Pattern types MVP #107606
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @jackh726 (or someone else) soon. Please see the contribution instructions for more information. |
Some changes occurred to the CTFE / Miri engine cc @rust-lang/miri
cc @davidtwco, @compiler-errors, @JohnTitor, @estebank, @TaKO8Ki Some changes occurred in src/tools/rustfmt cc @rust-lang/rustfmt rustdoc-json-types is a public (although nightly-only) API. If possible, consider changing cc @CraftSpider, @aDotInTheVoid, @Enselic, @obi1kenobi Some changes occurred in src/tools/clippy cc @rust-lang/clippy Some changes occurred in src/librustdoc/clean/types.rs cc @camelid Some changes occurred to the core trait solver cc @rust-lang/initiative-trait-system-refactor |
@@ -83,6 +83,9 @@ pub(crate) fn eval_nullary_intrinsic<'tcx>( | |||
ty::Alias(..) | ty::Param(_) | ty::Placeholder(_) | ty::Infer(_) => { | |||
throw_inval!(TooGeneric) | |||
} | |||
ty::Pat(..) => { | |||
unimplemented!("pattern types need to calculate pattern from their pattern") |
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.
My confusion about this error message remains.^^ Who's asking for a pattern to be computed from anything here? We just want a variant count! What does it even mean to compute a pattern from a pattern? Seems trivial, just use the identity function...
@@ -342,6 +342,12 @@ impl<'a> Parser<'a> { | |||
let span = lo.to(self.prev_token.span); | |||
let mut ty = self.mk_ty(span, kind); | |||
|
|||
if self.eat_keyword(sym::is) { |
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.
doesn't this change the behavior of macro code like:
macro_rules! foo {
($ty:ty) => {};
(() is $pat:pat) => {};
}
foo!(() is 0..1);
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.
macro rules matches continue being the least forwards compatible thing ever :|
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 it's better to expose this through a builtin macro instead of surface syntax, like rustc_pattern_type!()
which expands into ast::TyKind::Pat
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.
Or builtin#Pat(…)
instead of a built-in macro (with builtin#
from T-compiler MCP #580).
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.
macro rules matches continue being the least forwards compatible thing ever :|
And this is where the editions system can shine.
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'm a little surprised/sad that foo!
works at all today. Follow sets and greedy parsing were supposed to handle this -- you're not allowed to follow a :ty
fragment with is
-- but follow set restrictions are only applied within an arm, and not across arms. Being able to follow a fragment with some token in a simultaneously-taken arm matching the fragment's tokens essentially makes follow sets ineffectual at their job of making fragments forwards compatible. But simultaneously, applying follow set rules between arms would be very restrictive and probably not even solve the problem entirely, since you can attack non-follow-set grammar extensions similarly. I guess you'd need to ensure that the choice of which arm to take is always known to be forwards-compatibly LL(k) via some sort of "leading set," and if the choice isn't LL(k), ensure that the refining match fits within the grammar and follow set of the refined fragment.
i.e., complicated. And still overly restrictive, since we also have the rule for lexically prior arms taking priority over latter arms, so there's only a forwards compatibility concern if the refining arms come after the refined arm. (And until today I thought that macro parsing was greedier than it actually is, preventing foo!
from ever matching the second arm here, since the $:ty
arm is prior and matched the prefix. Despite probably relying on that not being the case.)
ast::TyKind::Pat(ref ty, ref pat) => { | ||
let ty = ty.rewrite(context, shape)?; | ||
let pat = pat.rewrite(context, shape)?; | ||
Some(format!("{ty} is {pat}")) | ||
}, |
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.
cc @rust-lang/style
This seems quite reasonable and noncontroversial to me, and I'd be surprised if there's any vehement objections to this formatting. However, want to make sure everyone has seen it in case we'd like a chance to discuss this as a team first.
In the event we do want to review as a team to identify and codify the formatting rules, then this can be changed to re-emit the original input contents in the interim via something like:
ast::TyKind::Pat(ref ty, ref pat) => { | |
let ty = ty.rewrite(context, shape)?; | |
let pat = pat.rewrite(context, shape)?; | |
Some(format!("{ty} is {pat}")) | |
}, | |
ast::TyKind::Pat(..) => Some(context.snippet(self.span).to_owned()), |
hir_analysis_pattern_type_wild_pat = "wildcard patterns are not permitted for pattern types" | ||
.label = "this type is the same as the inner type without a pattern" | ||
|
||
hir_analysis_pattern_type_non_const_range = "range patterns must have constant range start and end" |
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.
hir_analysis_pattern_type_wild_pat = "wildcard patterns are not permitted for pattern types" | |
.label = "this type is the same as the inner type without a pattern" | |
hir_analysis_pattern_type_non_const_range = "range patterns must have constant range start and end" | |
hir_analysis_pattern_type_wild_pat = wildcard patterns are not permitted for pattern types | |
.label = this type is the same as the inner type without a pattern | |
hir_analysis_pattern_type_non_const_range = range patterns must have constant range start and end |
Not asking for it to be implemented in this PR, but is the idea that these will have subtyping or coercions to less-restrictive patterns? Is I think I'm in favour, but it'd be nice to have a link to a post or something about a sketch of the goal here. |
What if types with patterns didn't have niches on their own, but were subtypes of the unbounded type? This would solve construction and usage: Construction is accomplished by matching the unbounded type against the pattern: match value as i32 {
x @ 0.. => Some(x), // x has type `i32 is 0..`
_ => None,
} Since Only if the pattern type is explicitly used in a field does the niche get exposed: struct NonZeroI32(i32 is ..0 | 1..);
println!("{}", size_of::<Option<NonZeroI32>>()); // Prints "4"
println!("{}", size_of::<Option<i32 is ..0 | 1..>>(); // Prints "5" Since the field is not generic, the subtyping information is hidden, so having a niche is sound. |
I am currently fiddling with rustc as a first project to implement way to make subenum (only at the declaration of the enum tho). This is kinda the same thing but with a broader range. I was thinking of allowing a sort of "specialization" where you can override function that exists in the parent enum. it would allow for some optimization. If I understand correctly these "pattern restricted types" are just for type checking and the layout right ? Would it be possible to add this functionnality to this or not (I doubt it since in my approch the subenums are declared WITH the main enum, and no consumer can create new subenum, so no orphan rule applies). |
Just some drive by prior art - this somewhat reminds me of ada pre & post conditions1, which I've thought was a cool feature that could potentially fit in where clauses. Basically it just adds a runtime check (or compile time) for the arguments and for the function results, and panics if it's not true (for things like train/plane control systems and other high-SIL applications) function Area(C: Circle)
with Pre => C.X_Coord – C.Radius > 0.0,
Post => Area'Result > 3.1 * C.Radius**2 and
Area'Result < 3.2 * C.Radius**2; Of course this is really more like the type constraints23 that Ada also has, but just a bit of an interesting related note. Footnotes |
this does support |
I wrote "Where do I pick up a shovel and help?" four months ago. There has been no answer in the intervening period. |
There's related discussion in https://rust-lang.zulipchat.com/#narrow/stream/144729-t-types/topic/Implementing.20Enum.20Variant.20Types/near/322878006 (TLDR: someone is working on reviving this PR with a placeholder macro for syntax (to eliminate the parser/macro issues this PR has). I don't know what the status of that work is, but maybe you two can collaborate? |
please open a types team MCP before trying to merge anything in this area (even if unstable). The exact way this new type kind should work is non-trivial and may have complex interactions with the rest of the type system. |
Apropos, keeping this PR open serves no purpose :D I'm closing it, we can continue the discussion on zulip |
Don't forget that the lifetime on hir types is `'tcx` This PR just tracks the `'tcx` lifetime to wherever the original objects actually have that lifetime. This code is needed for rust-lang#107606 (now rust-lang#120131) so that `ast_ty_to_ty` can invoke `lit_to_const` on an argument passed to it. Currently the argument is `&hir::Ty<'_>`, but after this PR it is `&'tcx hir::Ty<'tcx>`.
Don't forget that the lifetime on hir types is `'tcx` This PR just tracks the `'tcx` lifetime to wherever the original objects actually have that lifetime. This code is needed for rust-lang#107606 (now rust-lang#120131) so that `ast_ty_to_ty` can invoke `lit_to_const` on an argument passed to it. Currently the argument is `&hir::Ty<'_>`, but after this PR it is `&'tcx hir::Ty<'tcx>`.
Don't forget that the lifetime on hir types is `'tcx` This PR just tracks the `'tcx` lifetime to wherever the original objects actually have that lifetime. This code is needed for rust-lang/rust#107606 (now #120131) so that `ast_ty_to_ty` can invoke `lit_to_const` on an argument passed to it. Currently the argument is `&hir::Ty<'_>`, but after this PR it is `&'tcx hir::Ty<'tcx>`.
Don't forget that the lifetime on hir types is `'tcx` This PR just tracks the `'tcx` lifetime to wherever the original objects actually have that lifetime. This code is needed for rust-lang/rust#107606 (now #120131) so that `ast_ty_to_ty` can invoke `lit_to_const` on an argument passed to it. Currently the argument is `&hir::Ty<'_>`, but after this PR it is `&'tcx hir::Ty<'tcx>`.
…iler-errors Implement minimal, internal-only pattern types in the type system rebase of rust-lang#107606 You can create pattern types with `std::pat::pattern_type!(ty is pat)`. The feature is incomplete and will panic on you if you use any pattern other than integral range patterns. The only way to create or deconstruct a pattern type is via `transmute`. This PR's implementation differs from the MCP's text. Specifically > This means you could implement different traits for different pattern types with the same base type. Thus, we just forbid implementing any traits for pattern types. is violated in this PR. The reason is that we do need impls after all in order to make them usable as fields. constants of type `std::time::Nanoseconds` struct are used in patterns, so the type must be structural-eq, which it only can be if you derive several traits on it. It doesn't need to be structural-eq recursively, so we can just manually implement the relevant traits on the pattern type and use the pattern type as a private field. Waiting on: * [x] move all unrelated commits into their own PRs. * [x] fix niche computation (see 2db07f9) * [x] add lots more tests * [x] T-types MCP rust-lang/types-team#126 to finish * [x] some commit cleanup * [x] full self-review * [x] remove 61bd325, it's not necessary anymore I think. * [ ] ~~make sure we never accidentally leak pattern types to user code (add stability checks or feature gate checks and appopriate tests)~~ we don't even do this for the new float primitives * [x] get approval that [the scope expansion to trait impls](https://rust-lang.zulipchat.com/#narrow/stream/326866-t-types.2Fnominated/topic/Pattern.20types.20types-team.23126/near/427670099) is ok r? `@BoxyUwU`
Don't forget that the lifetime on hir types is `'tcx` This PR just tracks the `'tcx` lifetime to wherever the original objects actually have that lifetime. This code is needed for rust-lang/rust#107606 (now #120131) so that `ast_ty_to_ty` can invoke `lit_to_const` on an argument passed to it. Currently the argument is `&hir::Ty<'_>`, but after this PR it is `&'tcx hir::Ty<'tcx>`.
…iler-errors Implement minimal, internal-only pattern types in the type system rebase of rust-lang#107606 You can create pattern types with `std::pat::pattern_type!(ty is pat)`. The feature is incomplete and will panic on you if you use any pattern other than integral range patterns. The only way to create or deconstruct a pattern type is via `transmute`. This PR's implementation differs from the MCP's text. Specifically > This means you could implement different traits for different pattern types with the same base type. Thus, we just forbid implementing any traits for pattern types. is violated in this PR. The reason is that we do need impls after all in order to make them usable as fields. constants of type `std::time::Nanoseconds` struct are used in patterns, so the type must be structural-eq, which it only can be if you derive several traits on it. It doesn't need to be structural-eq recursively, so we can just manually implement the relevant traits on the pattern type and use the pattern type as a private field. Waiting on: * [x] move all unrelated commits into their own PRs. * [x] fix niche computation (see 2db07f9) * [x] add lots more tests * [x] T-types MCP rust-lang/types-team#126 to finish * [x] some commit cleanup * [x] full self-review * [x] remove 61bd325, it's not necessary anymore I think. * [ ] ~~make sure we never accidentally leak pattern types to user code (add stability checks or feature gate checks and appopriate tests)~~ we don't even do this for the new float primitives * [x] get approval that [the scope expansion to trait impls](https://rust-lang.zulipchat.com/#narrow/stream/326866-t-types.2Fnominated/topic/Pattern.20types.20types-team.23126/near/427670099) is ok r? `@BoxyUwU`
…iler-errors Implement minimal, internal-only pattern types in the type system rebase of rust-lang#107606 You can create pattern types with `std::pat::pattern_type!(ty is pat)`. The feature is incomplete and will panic on you if you use any pattern other than integral range patterns. The only way to create or deconstruct a pattern type is via `transmute`. This PR's implementation differs from the MCP's text. Specifically > This means you could implement different traits for different pattern types with the same base type. Thus, we just forbid implementing any traits for pattern types. is violated in this PR. The reason is that we do need impls after all in order to make them usable as fields. constants of type `std::time::Nanoseconds` struct are used in patterns, so the type must be structural-eq, which it only can be if you derive several traits on it. It doesn't need to be structural-eq recursively, so we can just manually implement the relevant traits on the pattern type and use the pattern type as a private field. Waiting on: * [x] move all unrelated commits into their own PRs. * [x] fix niche computation (see 2db07f9) * [x] add lots more tests * [x] T-types MCP rust-lang/types-team#126 to finish * [x] some commit cleanup * [x] full self-review * [x] remove 61bd325, it's not necessary anymore I think. * [ ] ~~make sure we never accidentally leak pattern types to user code (add stability checks or feature gate checks and appopriate tests)~~ we don't even do this for the new float primitives * [x] get approval that [the scope expansion to trait impls](https://rust-lang.zulipchat.com/#narrow/stream/326866-t-types.2Fnominated/topic/Pattern.20types.20types-team.23126/near/427670099) is ok r? `@BoxyUwU`
Implement minimal, internal-only pattern types in the type system rebase of rust-lang/rust#107606 You can create pattern types with `std::pat::pattern_type!(ty is pat)`. The feature is incomplete and will panic on you if you use any pattern other than integral range patterns. The only way to create or deconstruct a pattern type is via `transmute`. This PR's implementation differs from the MCP's text. Specifically > This means you could implement different traits for different pattern types with the same base type. Thus, we just forbid implementing any traits for pattern types. is violated in this PR. The reason is that we do need impls after all in order to make them usable as fields. constants of type `std::time::Nanoseconds` struct are used in patterns, so the type must be structural-eq, which it only can be if you derive several traits on it. It doesn't need to be structural-eq recursively, so we can just manually implement the relevant traits on the pattern type and use the pattern type as a private field. Waiting on: * [x] move all unrelated commits into their own PRs. * [x] fix niche computation (see 2db07f94f44f078daffe5823680d07d4fded883f) * [x] add lots more tests * [x] T-types MCP rust-lang/types-team#126 to finish * [x] some commit cleanup * [x] full self-review * [x] remove 61bd325da19a918cc3e02bbbdce97281a389c648, it's not necessary anymore I think. * [ ] ~~make sure we never accidentally leak pattern types to user code (add stability checks or feature gate checks and appopriate tests)~~ we don't even do this for the new float primitives * [x] get approval that [the scope expansion to trait impls](https://rust-lang.zulipchat.com/#narrow/stream/326866-t-types.2Fnominated/topic/Pattern.20types.20types-team.23126/near/427670099) is ok r? `@BoxyUwU`
Don't forget that the lifetime on hir types is `'tcx` This PR just tracks the `'tcx` lifetime to wherever the original objects actually have that lifetime. This code is needed for rust-lang/rust#107606 (now #120131) so that `ast_ty_to_ty` can invoke `lit_to_const` on an argument passed to it. Currently the argument is `&hir::Ty<'_>`, but after this PR it is `&'tcx hir::Ty<'tcx>`.
Doing #107299 the right way ™️
cc @joshtriplett for the language side
This PR adds pattern types to the parser and a minimal version to the type system. This isn't useful yet, as you can't create values of pattern types type except via
transmute
, but I thought it best to land this MVP and then iterate on the feature instead of producing a bigger and bigger PR that becomes harder to review with every addition.