Fix validation when lowering `?` trait bounds #132209

compiler-errors · 2024-10-27T00:41:08Z

Pass the unlowered (rustc_hir) polarity to lower_poly_trait_ref.

This allows us to actually validate that generic args are actually valid on ?Trait paths. This actually regressed in #113671 because that PR changed the behavior where we were inadvertently re-lowering paths as BoundPolarity::Positive, which was also coincidentally the only place we were enforcing the generics on ?Trait paths were correct.

rustbot · 2024-10-27T00:41:16Z

r? @Nadrieril

rustbot has assigned @Nadrieril.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

rustbot · 2024-10-27T00:41:18Z

HIR ty lowering was modified

cc @fmease

compiler-errors · 2024-10-27T00:41:26Z

Funny enough, this code passes on nightly:

fn uwu<T: ?Sized<i32>>() {}

compiler-errors · 2024-10-27T00:45:00Z

compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs

@@ -681,11 +681,20 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
            Some(self_ty),


^(a handful of lines above) we were probably also allowing ?std::ops::Fn<T> but i dont care to write a test for that.

compiler-errors · 2024-10-27T00:57:04Z

Frankly I am of half a mind to say that we don't need to do the annoying and time-consuming dance of cratering the regression for (2.). If this actually ended up being "used" anywhere in the wild, then we will obviously catch this in the next beta crater and deal with it then, or I can crater this after it's landed.

Nadrieril · 2024-10-27T09:38:20Z

The span change I can approve; the validation change looks good but I don't know that code, @fmease any objections?

compiler-errors · 2024-10-27T15:38:25Z

Actually, I'll just split the span change out and just r=Nadrieril on that one first, since it's really kinda unrelated. My fault for putting this into one PR rbh.

… r=Nadrieril Pass constness with span into lower_poly_trait_ref Gives us a span to point at for ~const/const on non-const traits. Split from rust-lang#132209. r? Nadrieril

Rollup merge of rust-lang#132227 - compiler-errors:better-const-span, r=Nadrieril Pass constness with span into lower_poly_trait_ref Gives us a span to point at for ~const/const on non-const traits. Split from rust-lang#132209. r? Nadrieril

fmease · 2024-10-30T23:41:36Z

This actually regressed in #113671 because that PR changed the behavior where we were inadvertently re-lowering paths as BoundPolarity::Positive, which was also coincidentally the only place we were enforcing the generics on ?Trait paths were correct.

Omg, this is insane! 🙏 Thanks for catching this!

fmease

I have one concern but otherwise looks pretty good to me!

fmease · 2024-10-30T23:39:24Z

compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs

+                // No-op.
+                return arg_count;


Doesn't this early return mean we don't validate associated item constraints which get lowered below this point in lower_assoc_item_constraint? E.g., isn't it still the case that ?Sized<Undefined = (), Undefined = ()> gets accepted under this PR?

OK, so I made it so that we at least validate these associated type bounds, but not outright deny them. We may want to do that later.

compiler-errors · 2024-10-31T01:21:18Z

@rustbot ready

fmease · 2024-10-31T16:42:42Z

@bors r+

bors · 2024-10-31T16:42:45Z

📌 Commit 06a49b6 has been approved by fmease

It is now in the queue for this repository.

Fix validation when lowering `?` trait bounds Pass the unlowered (`rustc_hir`) polarity to `lower_poly_trait_ref`. This allows us to actually *validate* that generic args are actually valid on `?Trait` paths. This actually regressed in rust-lang#113671 because that PR changed the behavior where we were inadvertently re-lowering paths as `BoundPolarity::Positive`, which was also coincidentally the only place we were enforcing the generics on `?Trait` paths were correct.

…iaskrgr Rollup of 5 pull requests Successful merges: - rust-lang#131168 (Fix `target_os` for `mipsel-sony-psx`) - rust-lang#132209 (Fix validation when lowering `?` trait bounds) - rust-lang#132357 (Improve missing_abi lint) - rust-lang#132385 (compiler: Move `rustc_target::spec::abi::Abi` to `rustc_abi::ExternAbi`) - rust-lang#132417 (macOS: Document the difference between Clang's `-darwin` and `-macosx` targets) r? `@ghost` `@rustbot` modify labels: rollup

…kingjubilee Rollup of 9 pull requests Successful merges: - rust-lang#131168 (Fix `target_os` for `mipsel-sony-psx`) - rust-lang#132209 (Fix validation when lowering `?` trait bounds) - rust-lang#132294 (Bump Fuchsia) - rust-lang#132357 (Improve missing_abi lint) - rust-lang#132385 (compiler: Move `rustc_target::spec::abi::Abi` to `rustc_abi::ExternAbi`) - rust-lang#132403 (continue `TypingMode` refactor) - rust-lang#132417 (macOS: Document the difference between Clang's `-darwin` and `-macosx` targets) - rust-lang#132421 (Remove `""` case from RISC-V `llvm_abiname` match statement) - rust-lang#132422 (llvm: Match new LLVM 128-bit integer alignment on sparc) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#132209 - compiler-errors:modifiers, r=fmease Fix validation when lowering `?` trait bounds Pass the unlowered (`rustc_hir`) polarity to `lower_poly_trait_ref`. This allows us to actually *validate* that generic args are actually valid on `?Trait` paths. This actually regressed in rust-lang#113671 because that PR changed the behavior where we were inadvertently re-lowering paths as `BoundPolarity::Positive`, which was also coincidentally the only place we were enforcing the generics on `?Trait` paths were correct.

rustbot assigned Nadrieril Oct 27, 2024

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 27, 2024

compiler-errors commented Oct 27, 2024

View reviewed changes

compiler-errors changed the title ~~Improve lowering of poly trait refs~~ Fix validation when lowering ? trait bounds, improve spans for const trait bounds Oct 27, 2024

compiler-errors mentioned this pull request Oct 27, 2024

Pass constness with span into lower_poly_trait_ref #132227

Merged

fmease assigned fmease and unassigned Nadrieril Oct 27, 2024

compiler-errors force-pushed the modifiers branch 3 times, most recently from 0cdc673 to c8f33e5 Compare October 30, 2024 02:50

fmease changed the title ~~Fix validation when lowering ? trait bounds, improve spans for const trait bounds~~ Fix validation when lowering ? trait bounds Oct 30, 2024

fmease requested changes Oct 30, 2024

View reviewed changes

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 30, 2024

compiler-errors added 2 commits October 30, 2024 23:42

Actually do validation for poly trait refs with ? modifier

e356279

Validate associated type bounds on ?

06a49b6

compiler-errors force-pushed the modifiers branch from c8f33e5 to 06a49b6 Compare October 31, 2024 00:09

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 31, 2024

fmease approved these changes Oct 31, 2024

View reviewed changes

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 31, 2024

matthiaskrgr mentioned this pull request Oct 31, 2024

Rollup of 5 pull requests #132420

Closed

workingjubilee mentioned this pull request Nov 1, 2024

Rollup of 9 pull requests #132435

Merged

bors merged commit e31988c into rust-lang:master Nov 1, 2024
6 checks passed

rustbot added this to the 1.84.0 milestone Nov 1, 2024

compiler-errors mentioned this pull request Nov 1, 2024

Unsoundness: Rust since 1.78 accepts invalid ? #132463

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix validation when lowering `?` trait bounds #132209

Fix validation when lowering `?` trait bounds #132209

compiler-errors commented Oct 27, 2024 •

edited by fmease

Loading

rustbot commented Oct 27, 2024

rustbot commented Oct 27, 2024

compiler-errors commented Oct 27, 2024

compiler-errors Oct 27, 2024 •

edited

Loading

compiler-errors commented Oct 27, 2024

Nadrieril commented Oct 27, 2024

compiler-errors commented Oct 27, 2024

fmease commented Oct 30, 2024

fmease left a comment •

edited

Loading

fmease Oct 30, 2024 •

edited

Loading

compiler-errors Oct 30, 2024

compiler-errors Oct 31, 2024

compiler-errors commented Oct 31, 2024

fmease commented Oct 31, 2024

bors commented Oct 31, 2024

		@@ -681,11 +681,20 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
		Some(self_ty),

Fix validation when lowering ? trait bounds #132209

Fix validation when lowering ? trait bounds #132209

Conversation

compiler-errors commented Oct 27, 2024 • edited by fmease Loading

rustbot commented Oct 27, 2024

rustbot commented Oct 27, 2024

compiler-errors commented Oct 27, 2024

compiler-errors Oct 27, 2024 • edited Loading

Choose a reason for hiding this comment

compiler-errors commented Oct 27, 2024

Nadrieril commented Oct 27, 2024

compiler-errors commented Oct 27, 2024

fmease commented Oct 30, 2024

fmease left a comment • edited Loading

Choose a reason for hiding this comment

fmease Oct 30, 2024 • edited Loading

Choose a reason for hiding this comment

compiler-errors Oct 30, 2024

Choose a reason for hiding this comment

compiler-errors Oct 31, 2024

Choose a reason for hiding this comment

compiler-errors commented Oct 31, 2024

fmease commented Oct 31, 2024

bors commented Oct 31, 2024

Fix validation when lowering `?` trait bounds #132209

Fix validation when lowering `?` trait bounds #132209

compiler-errors commented Oct 27, 2024 •

edited by fmease

Loading

compiler-errors Oct 27, 2024 •

edited

Loading

fmease left a comment •

edited

Loading

fmease Oct 30, 2024 •

edited

Loading