Skip to content

Commit

Permalink
Rollup merge of rust-lang#132209 - compiler-errors:modifiers, r=fmease
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
matthiaskrgr authored Oct 31, 2024
2 parents b6a7b18 + 06a49b6 commit 268f259
Show file tree
Hide file tree
Showing 13 changed files with 119 additions and 58 deletions.
6 changes: 0 additions & 6 deletions compiler/rustc_hir_analysis/src/hir_ty_lowering/bounds.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,12 +168,6 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
match hir_bound {
hir::GenericBound::Trait(poly_trait_ref) => {
let hir::TraitBoundModifiers { constness, polarity } = poly_trait_ref.modifiers;
let polarity = match polarity {
rustc_ast::BoundPolarity::Positive => ty::PredicatePolarity::Positive,
rustc_ast::BoundPolarity::Negative(_) => ty::PredicatePolarity::Negative,
rustc_ast::BoundPolarity::Maybe(_) => continue,
};

let _ = self.lower_poly_trait_ref(
&poly_trait_ref.trait_ref,
poly_trait_ref.span,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
&trait_bound.trait_ref,
trait_bound.span,
hir::BoundConstness::Never,
ty::PredicatePolarity::Positive,
hir::BoundPolarity::Positive,
dummy_self,
&mut bounds,
PredicateFilter::SelfOnly,
Expand Down
47 changes: 34 additions & 13 deletions compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -668,7 +668,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
trait_ref: &hir::TraitRef<'tcx>,
span: Span,
constness: hir::BoundConstness,
polarity: ty::PredicatePolarity,
polarity: hir::BoundPolarity,
self_ty: Ty<'tcx>,
bounds: &mut Bounds<'tcx>,
predicate_filter: PredicateFilter,
Expand All @@ -690,15 +690,6 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
Some(self_ty),
);

if let hir::BoundConstness::Always(span) | hir::BoundConstness::Maybe(span) = constness
&& !self.tcx().is_const_trait(trait_def_id)
{
self.dcx().emit_err(crate::errors::ConstBoundForNonConstTrait {
span,
modifier: constness.as_str(),
});
}

let tcx = self.tcx();
let bound_vars = tcx.late_bound_vars(trait_ref.hir_ref_id);
debug!(?bound_vars);
Expand All @@ -708,6 +699,36 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
bound_vars,
);

let polarity = match polarity {
rustc_ast::BoundPolarity::Positive => ty::PredicatePolarity::Positive,
rustc_ast::BoundPolarity::Negative(_) => ty::PredicatePolarity::Negative,
rustc_ast::BoundPolarity::Maybe(_) => {
// Validate associated type at least. We may want to reject these
// outright in the future...
for constraint in trait_segment.args().constraints {
let _ = self.lower_assoc_item_constraint(
trait_ref.hir_ref_id,
poly_trait_ref,
constraint,
&mut Default::default(),
&mut Default::default(),
constraint.span,
predicate_filter,
);
}
return arg_count;
}
};

if let hir::BoundConstness::Always(span) | hir::BoundConstness::Maybe(span) = constness
&& !self.tcx().is_const_trait(trait_def_id)
{
self.dcx().emit_err(crate::errors::ConstBoundForNonConstTrait {
span,
modifier: constness.as_str(),
});
}

match predicate_filter {
PredicateFilter::All
| PredicateFilter::SelfOnly
Expand Down Expand Up @@ -763,11 +784,11 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
// since we should have emitted an error for them earlier, and they
// would not be well-formed!
if polarity != ty::PredicatePolarity::Positive {
assert!(
self.dcx().has_errors().is_some(),
self.dcx().span_delayed_bug(
constraint.span,
"negative trait bounds should not have assoc item constraints",
);
continue;
break;
}

// Specify type to assert that error was already reported in `Err` case.
Expand Down
2 changes: 0 additions & 2 deletions tests/ui/impl-trait/precise-capturing/bound-modifiers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@
fn polarity() -> impl Sized + ?use<> {}
//~^ ERROR expected identifier, found keyword `use`
//~| ERROR cannot find trait `r#use` in this scope
//~| WARN relaxing a default bound only does something for `?Sized`
//~| WARN relaxing a default bound only does something for `?Sized`

fn asyncness() -> impl Sized + async use<> {}
//~^ ERROR expected identifier, found keyword `use`
Expand Down
32 changes: 9 additions & 23 deletions tests/ui/impl-trait/precise-capturing/bound-modifiers.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,19 @@ LL | fn polarity() -> impl Sized + ?use<> {}
| ^^^ expected identifier, found keyword

error: expected identifier, found keyword `use`
--> $DIR/bound-modifiers.rs:9:38
--> $DIR/bound-modifiers.rs:7:38
|
LL | fn asyncness() -> impl Sized + async use<> {}
| ^^^ expected identifier, found keyword

error: expected identifier, found keyword `use`
--> $DIR/bound-modifiers.rs:14:38
--> $DIR/bound-modifiers.rs:12:38
|
LL | fn constness() -> impl Sized + const use<> {}
| ^^^ expected identifier, found keyword

error: expected identifier, found keyword `use`
--> $DIR/bound-modifiers.rs:19:37
--> $DIR/bound-modifiers.rs:17:37
|
LL | fn binder() -> impl Sized + for<'a> use<> {}
| ^^^ expected identifier, found keyword
Expand All @@ -29,25 +29,25 @@ LL | fn polarity() -> impl Sized + ?use<> {}
| ^^^ not found in this scope

error[E0405]: cannot find trait `r#use` in this scope
--> $DIR/bound-modifiers.rs:9:38
--> $DIR/bound-modifiers.rs:7:38
|
LL | fn asyncness() -> impl Sized + async use<> {}
| ^^^ not found in this scope

error[E0405]: cannot find trait `r#use` in this scope
--> $DIR/bound-modifiers.rs:14:38
--> $DIR/bound-modifiers.rs:12:38
|
LL | fn constness() -> impl Sized + const use<> {}
| ^^^ not found in this scope

error[E0405]: cannot find trait `r#use` in this scope
--> $DIR/bound-modifiers.rs:19:37
--> $DIR/bound-modifiers.rs:17:37
|
LL | fn binder() -> impl Sized + for<'a> use<> {}
| ^^^ not found in this scope

error[E0658]: async closures are unstable
--> $DIR/bound-modifiers.rs:9:32
--> $DIR/bound-modifiers.rs:7:32
|
LL | fn asyncness() -> impl Sized + async use<> {}
| ^^^^^
Expand All @@ -58,7 +58,7 @@ LL | fn asyncness() -> impl Sized + async use<> {}
= help: to use an async block, remove the `||`: `async {`

error[E0658]: const trait impls are experimental
--> $DIR/bound-modifiers.rs:14:32
--> $DIR/bound-modifiers.rs:12:32
|
LL | fn constness() -> impl Sized + const use<> {}
| ^^^^^
Expand All @@ -67,21 +67,7 @@ LL | fn constness() -> impl Sized + const use<> {}
= help: add `#![feature(const_trait_impl)]` to the crate attributes to enable
= note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date

warning: relaxing a default bound only does something for `?Sized`; all other traits are not bound by default
--> $DIR/bound-modifiers.rs:3:31
|
LL | fn polarity() -> impl Sized + ?use<> {}
| ^^^^^^

warning: relaxing a default bound only does something for `?Sized`; all other traits are not bound by default
--> $DIR/bound-modifiers.rs:3:31
|
LL | fn polarity() -> impl Sized + ?use<> {}
| ^^^^^^
|
= note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`

error: aborting due to 10 previous errors; 2 warnings emitted
error: aborting due to 10 previous errors

Some errors have detailed explanations: E0405, E0658.
For more information about an error, try `rustc --explain E0405`.
3 changes: 1 addition & 2 deletions tests/ui/issues/issue-37534.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
struct Foo<T: ?Hash> {}
//~^ ERROR expected trait, found derive macro `Hash`
//~^^ ERROR parameter `T` is never used
//~^^^ WARN relaxing a default bound only does something for `?Sized`
//~| WARN relaxing a default bound only does something for `?Sized`

fn main() {}
13 changes: 2 additions & 11 deletions tests/ui/issues/issue-37534.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,6 @@ warning: relaxing a default bound only does something for `?Sized`; all other tr
LL | struct Foo<T: ?Hash> {}
| ^^^^^

error[E0392]: type parameter `T` is never used
--> $DIR/issue-37534.rs:1:12
|
LL | struct Foo<T: ?Hash> {}
| ^ unused type parameter
|
= help: consider removing `T`, referring to it in a field, or using a marker such as `PhantomData`

error: aborting due to 2 previous errors; 1 warning emitted
error: aborting due to 1 previous error; 1 warning emitted

Some errors have detailed explanations: E0392, E0404.
For more information about an error, try `rustc --explain E0392`.
For more information about this error, try `rustc --explain E0404`.
4 changes: 4 additions & 0 deletions tests/ui/trait-bounds/maybe-bound-generics-deny.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
fn uwu<T: ?Sized<i32>>() {}
//~^ ERROR trait takes 0 generic arguments but 1 generic argument was supplied

fn main() {}
11 changes: 11 additions & 0 deletions tests/ui/trait-bounds/maybe-bound-generics-deny.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
error[E0107]: trait takes 0 generic arguments but 1 generic argument was supplied
--> $DIR/maybe-bound-generics-deny.rs:1:12
|
LL | fn uwu<T: ?Sized<i32>>() {}
| ^^^^^----- help: remove the unnecessary generics
| |
| expected 0 generic arguments

error: aborting due to 1 previous error

For more information about this error, try `rustc --explain E0107`.
7 changes: 7 additions & 0 deletions tests/ui/trait-bounds/maybe-bound-has-path-args.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
trait Trait {}

fn test<T: ?self::<i32>::Trait>() {}
//~^ ERROR type arguments are not allowed on this type
//~| WARN relaxing a default bound only does something for `?Sized`

fn main() {}
17 changes: 17 additions & 0 deletions tests/ui/trait-bounds/maybe-bound-has-path-args.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
warning: relaxing a default bound only does something for `?Sized`; all other traits are not bound by default
--> $DIR/maybe-bound-has-path-args.rs:3:12
|
LL | fn test<T: ?self::<i32>::Trait>() {}
| ^^^^^^^^^^^^^^^^^^^

error[E0109]: type arguments are not allowed on this type
--> $DIR/maybe-bound-has-path-args.rs:3:20
|
LL | fn test<T: ?self::<i32>::Trait>() {}
| ---- ^^^ type argument not allowed
| |
| not allowed on this type

error: aborting due to 1 previous error; 1 warning emitted

For more information about this error, try `rustc --explain E0109`.
12 changes: 12 additions & 0 deletions tests/ui/trait-bounds/maybe-bound-with-assoc.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
trait HasAssoc {
type Assoc;
}
fn hasassoc<T: ?HasAssoc<Assoc = ()>>() {}
//~^ WARN relaxing a default bound

trait NoAssoc {}
fn noassoc<T: ?NoAssoc<Missing = ()>>() {}
//~^ WARN relaxing a default bound
//~| ERROR associated type `Missing` not found for `NoAssoc`

fn main() {}
21 changes: 21 additions & 0 deletions tests/ui/trait-bounds/maybe-bound-with-assoc.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
warning: relaxing a default bound only does something for `?Sized`; all other traits are not bound by default
--> $DIR/maybe-bound-with-assoc.rs:4:16
|
LL | fn hasassoc<T: ?HasAssoc<Assoc = ()>>() {}
| ^^^^^^^^^^^^^^^^^^^^^

warning: relaxing a default bound only does something for `?Sized`; all other traits are not bound by default
--> $DIR/maybe-bound-with-assoc.rs:8:15
|
LL | fn noassoc<T: ?NoAssoc<Missing = ()>>() {}
| ^^^^^^^^^^^^^^^^^^^^^^

error[E0220]: associated type `Missing` not found for `NoAssoc`
--> $DIR/maybe-bound-with-assoc.rs:8:24
|
LL | fn noassoc<T: ?NoAssoc<Missing = ()>>() {}
| ^^^^^^^ associated type `Missing` not found

error: aborting due to 1 previous error; 2 warnings emitted

For more information about this error, try `rustc --explain E0220`.

0 comments on commit 268f259

Please sign in to comment.