Allow using named consts in pattern types #136284
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Some changes occurred in src/tools/clippy cc @rust-lang/clippy HIR ty lowering was modified cc @fmease |
62ac839 to
5337e0d
Compare
|
☔ The latest upstream changes (presumably #136454) made this pull request unmergeable. Please resolve the merge conflicts. |
5337e0d to
611b966
Compare
lcnr
reviewed
Feb 3, 2025
| span: self.lower_span(pattern.span), | ||
| }); | ||
| hir::ConstArgKind::Anon(ac) | ||
| }; |
There was a problem hiding this comment.
this is pretty much mgce? 🤔 as in, this also adds consts in the type system.
Can you delegate this to lower_const_arg or whatever instead?
There was a problem hiding this comment.
Yes, but at that point I should modify the ast, and I'd prefer to do that in other PRs
lcnr
reviewed
Feb 3, 2025
| let hir::TyKind::Pat(ty, p) = tcx.parent_hir_node(pat.hir_id).expect_ty().kind else { | ||
| bug!() | ||
| }; | ||
| assert_eq!(p.hir_id, pat.hir_id); |
There was a problem hiding this comment.
why do we use the same HirId for the TyKind::Pat and its pattern?
There was a problem hiding this comment.
We don't. This assertion is just x == child(parent(x)) and mostly a leftover from getting the PR to this state
lcnr
reviewed
Feb 3, 2025
| let hir::TyKind::Pat(ty, p) = tcx.parent_hir_node(pat.hir_id).expect_ty().kind else { | ||
| bug!() | ||
| }; | ||
| assert_eq!(p.hir_id, pat.hir_id); |
| let start = start.map(expr_to_const); | ||
| let end = end.map(expr_to_const); | ||
| let start = start.map(|expr| self.lower_const_arg(expr, FeedConstTy::No)); | ||
| let end = end.map(|expr| self.lower_const_arg(expr, FeedConstTy::No)); |
There was a problem hiding this comment.
do we currently not evaluate the range during typeck?
given that we don't feed its types and computing the type relies on typeck?
| @@ -0,0 +1,72 @@ | |||
| error[E0391]: cycle detected when evaluating type-level constant | |||
There was a problem hiding this comment.
i guess the answer is: we do evaluate them during typeck and just get cycle errors for now :3
611b966 to
fbcaa9b
Compare
|
@bors r=lcnr |
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Feb 4, 2025
…iaskrgr Rollup of 7 pull requests Successful merges: - rust-lang#136242 (Remove `LateContext::match_def_path()`) - rust-lang#136274 (Check Sizedness of return type in WF) - rust-lang#136284 (Allow using named consts in pattern types) - rust-lang#136477 (Fix a couple NLL TLS spans ) - rust-lang#136497 (Report generic mismatches when calling bodyless trait functions) - rust-lang#136520 (Remove unnecessary layout assertions for object-safe receivers) - rust-lang#136526 (mir_build: Rename `thir::cx::Cx` to `ThirBuildCx` and remove `UserAnnotatedTyHelpers`) Failed merges: - rust-lang#136304 (Reject negative literals for unsigned or char types in pattern ranges and literals) r? `@ghost` `@rustbot` modify labels: rollup
rust-timer
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Feb 4, 2025
Rollup merge of rust-lang#136284 - oli-obk:push-zsxuwnzmonnl, r=lcnr Allow using named consts in pattern types This required a refactoring first: I had to stop using `hir::Pat`in `hir::TyKind::Pat` and instead create a separate `TyPat` that has `ConstArg` for range ends instead of `PatExpr`. Within the type system we should be using `ConstArg` for all constants, as otherwise we'd be maintaining two separate const systems that could diverge. The big advantage of this PR is that we now inherit all the rules from const generics and don't have a separate system. While this makes things harder for users (const generic rules wrt what is allowed in those consts), it also means we don't accidentally allow some things like referring to assoc consts or doing math on generic consts.
This was referenced Feb 5, 2025
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this pull request
Feb 11, 2025
Add a TyPat in the AST to reuse the generic arg lowering logic This simplifies ast lowering significantly with little cost to the pattern types parser. Also fixes any problems we've had with generic args (well, pushes any problems onto the `generic_const_exprs` feature gate) follow-up to rust-lang#136284 (comment) r? `@BoxyUwU`
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this pull request
Feb 12, 2025
Add a TyPat in the AST to reuse the generic arg lowering logic This simplifies ast lowering significantly with little cost to the pattern types parser. Also fixes any problems we've had with generic args (well, pushes any problems onto the `generic_const_exprs` feature gate) follow-up to rust-lang#136284 (comment) r? ``@BoxyUwU``
rust-timer
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Feb 12, 2025
Rollup merge of rust-lang#136646 - oli-obk:pattern-types-ast, r=BoxyUwU Add a TyPat in the AST to reuse the generic arg lowering logic This simplifies ast lowering significantly with little cost to the pattern types parser. Also fixes any problems we've had with generic args (well, pushes any problems onto the `generic_const_exprs` feature gate) follow-up to rust-lang#136284 (comment) r? ``@BoxyUwU``
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This required a refactoring first: I had to stop using
hir::Patinhir::TyKind::Patand instead create a separateTyPatthat hasConstArgfor range ends instead ofPatExpr. Within the type system we should be usingConstArgfor all constants, as otherwise we'd be maintaining two separate const systems that could diverge. The big advantage of this PR is that we now inherit all the rules from const generics and don't have a separate system. While this makes things harder for users (const generic rules wrt what is allowed in those consts), it also means we don't accidentally allow some things like referring to assoc consts or doing math on generic consts.