-
Notifications
You must be signed in to change notification settings - Fork 13.5k
More robustly deal with relaxed bounds and improve their diagnostics #142693
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
base: master
Are you sure you want to change the base?
Changes from 1 commit
af163d4
ee0b644
624f1f8
c619141
3cace54
6983b44
2028d25
45a63cb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -758,13 +758,36 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { | |
bounds: &mut Vec<(ty::Clause<'tcx>, Span)>, | ||
predicate_filter: PredicateFilter, | ||
) -> GenericArgCountResult { | ||
let tcx = self.tcx(); | ||
|
||
// We use the *resolved* bound vars later instead of the HIR ones since the former | ||
// also include the bound vars of the overarching predicate if applicable. | ||
let hir::PolyTraitRef { bound_generic_params: _, modifiers, ref trait_ref, span } = | ||
*poly_trait_ref; | ||
let hir::TraitBoundModifiers { constness, polarity } = modifiers; | ||
|
||
let trait_def_id = trait_ref.trait_def_id().unwrap_or_else(|| FatalError.raise()); | ||
|
||
let (polarity, bounds) = match polarity { | ||
rustc_ast::BoundPolarity::Positive | ||
if tcx.is_lang_item(trait_def_id, hir::LangItem::PointeeSized) => | ||
{ | ||
// Skip `PointeeSized` bounds. | ||
// | ||
// `PointeeSized` is a "fake bound" insofar as anywhere a `PointeeSized` bound exists, there | ||
// is actually the absence of any bounds. This avoids limitations around non-global where | ||
// clauses being preferred over item bounds (where `PointeeSized` bounds would be | ||
// proven) - which can result in errors when a `PointeeSized` supertrait/bound/predicate is | ||
// added to some items. | ||
(ty::PredicatePolarity::Positive, &mut Vec::new()) | ||
} | ||
rustc_ast::BoundPolarity::Positive => (ty::PredicatePolarity::Positive, bounds), | ||
rustc_ast::BoundPolarity::Negative(_) => (ty::PredicatePolarity::Negative, bounds), | ||
rustc_ast::BoundPolarity::Maybe(_) => { | ||
(ty::PredicatePolarity::Positive, &mut Vec::new()) | ||
} | ||
Comment on lines
+786
to
+788
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (repost of #142693 (comment) which is on an outdated diff) I'm not happy about the way this is structured and the fact that we map Bigger picture: As you can see a few lines further down we used to duplicate the Idk :| |
||
}; | ||
|
||
let trait_segment = trait_ref.path.segments.last().unwrap(); | ||
|
||
let _ = self.prohibit_generic_args( | ||
|
@@ -781,7 +804,6 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { | |
Some(self_ty), | ||
); | ||
|
||
let tcx = self.tcx(); | ||
let bound_vars = tcx.late_bound_vars(trait_ref.hir_ref_id); | ||
debug!(?bound_vars); | ||
|
||
|
@@ -792,27 +814,6 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { | |
|
||
debug!(?poly_trait_ref); | ||
|
||
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; | ||
} | ||
}; | ||
|
||
// We deal with const conditions later. | ||
match predicate_filter { | ||
PredicateFilter::All | ||
|
@@ -915,7 +916,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { | |
// Don't register any associated item constraints for negative bounds, | ||
// since we should have emitted an error for them earlier, and they | ||
// would not be well-formed! | ||
if polarity != ty::PredicatePolarity::Positive { | ||
if polarity == ty::PredicatePolarity::Negative { | ||
self.dcx().span_delayed_bug( | ||
constraint.span, | ||
"negative trait bounds should not have assoc item constraints", | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
// Test that despite us dropping `PointeeSized` bounds during HIR ty lowering | ||
// we still validate it first. | ||
// issue: <https://github.com/rust-lang/rust/issues/142718> | ||
#![feature(sized_hierarchy)] | ||
|
||
use std::marker::PointeeSized; | ||
|
||
struct T where (): PointeeSized<(), Undefined = ()>; | ||
//~^ ERROR trait takes 0 generic arguments but 1 generic argument was supplied | ||
//~| ERROR associated type `Undefined` not found for `PointeeSized` | ||
|
||
const fn test<T, U>() where T: const PointeeSized, U: [const] PointeeSized {} | ||
//~^ ERROR `const` can only be applied to `#[const_trait]` traits | ||
//~| ERROR `const` can only be applied to `#[const_trait]` traits | ||
//~| ERROR const trait impls are experimental | ||
//~| ERROR `[const]` can only be applied to `#[const_trait]` traits | ||
//~| ERROR `[const]` can only be applied to `#[const_trait]` traits | ||
//~| ERROR const trait impls are experimental | ||
|
||
fn main() {} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,76 @@ | ||
error[E0658]: const trait impls are experimental | ||
--> $DIR/pointee-validation.rs:12:32 | ||
| | ||
LL | const fn test<T, U>() where T: const PointeeSized, U: [const] PointeeSized {} | ||
| ^^^^^ | ||
| | ||
= note: see issue #67792 <https://github.com/rust-lang/rust/issues/67792> for more information | ||
= 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 | ||
|
||
error[E0658]: const trait impls are experimental | ||
--> $DIR/pointee-validation.rs:12:53 | ||
| | ||
LL | const fn test<T, U>() where T: const PointeeSized, U: [const] PointeeSized {} | ||
| ^^^^^^^^^ | ||
| | ||
= note: see issue #67792 <https://github.com/rust-lang/rust/issues/67792> for more information | ||
= 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 | ||
|
||
error[E0107]: trait takes 0 generic arguments but 1 generic argument was supplied | ||
--> $DIR/pointee-validation.rs:8:20 | ||
| | ||
LL | struct T where (): PointeeSized<(), Undefined = ()>; | ||
| ^^^^^^^^^^^^-------------------- help: remove the unnecessary generics | ||
| | | ||
| expected 0 generic arguments | ||
|
||
error[E0220]: associated type `Undefined` not found for `PointeeSized` | ||
--> $DIR/pointee-validation.rs:8:37 | ||
| | ||
LL | struct T where (): PointeeSized<(), Undefined = ()>; | ||
| ^^^^^^^^^ associated type `Undefined` not found | ||
|
||
error: `const` can only be applied to `#[const_trait]` traits | ||
--> $DIR/pointee-validation.rs:12:32 | ||
| | ||
LL | const fn test<T, U>() where T: const PointeeSized, U: [const] PointeeSized {} | ||
| ^^^^^ can't be applied to `PointeeSized` | ||
| | ||
note: `PointeeSized` can't be used with `const` because it isn't annotated with `#[const_trait]` | ||
--> $SRC_DIR/core/src/marker.rs:LL:COL | ||
|
||
error: `[const]` can only be applied to `#[const_trait]` traits | ||
--> $DIR/pointee-validation.rs:12:53 | ||
| | ||
LL | const fn test<T, U>() where T: const PointeeSized, U: [const] PointeeSized {} | ||
| ^^^^^^^^^ can't be applied to `PointeeSized` | ||
| | ||
note: `PointeeSized` can't be used with `[const]` because it isn't annotated with `#[const_trait]` | ||
--> $SRC_DIR/core/src/marker.rs:LL:COL | ||
|
||
error: `const` can only be applied to `#[const_trait]` traits | ||
--> $DIR/pointee-validation.rs:12:32 | ||
| | ||
LL | const fn test<T, U>() where T: const PointeeSized, U: [const] PointeeSized {} | ||
| ^^^^^ can't be applied to `PointeeSized` | ||
| | ||
note: `PointeeSized` can't be used with `const` because it isn't annotated with `#[const_trait]` | ||
--> $SRC_DIR/core/src/marker.rs:LL:COL | ||
= note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` | ||
|
||
error: `[const]` can only be applied to `#[const_trait]` traits | ||
--> $DIR/pointee-validation.rs:12:53 | ||
| | ||
LL | const fn test<T, U>() where T: const PointeeSized, U: [const] PointeeSized {} | ||
| ^^^^^^^^^ can't be applied to `PointeeSized` | ||
| | ||
note: `PointeeSized` can't be used with `[const]` because it isn't annotated with `#[const_trait]` | ||
--> $SRC_DIR/core/src/marker.rs:LL:COL | ||
= note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` | ||
|
||
error: aborting due to 8 previous errors | ||
|
||
Some errors have detailed explanations: E0107, E0220, E0658. | ||
For more information about an error, try `rustc --explain E0107`. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,14 +1,15 @@ | ||
error: `PointeeSized` cannot be used with trait objects | ||
error[E0224]: at least one trait is required for an object type | ||
--> $DIR/reject-dyn-pointeesized.rs:5:12 | ||
| | ||
LL | type Foo = dyn PointeeSized; | ||
| ^^^^^^^^^^^^^^^^ | ||
|
||
error: `PointeeSized` cannot be used with trait objects | ||
error[E0224]: at least one trait is required for an object type | ||
--> $DIR/reject-dyn-pointeesized.rs:14:16 | ||
| | ||
LL | let y: Box<dyn PointeeSized> = x; | ||
| ^^^^^^^^^^^^^^^^ | ||
|
||
error: aborting due to 2 previous errors | ||
|
||
For more information about this error, try `rustc --explain E0224`. |
Uh oh!
There was an error while loading. Please reload this page.
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 PR predates #143104.
This commit basically reverts this logic because it's unreachable due to us filtering out
PointeeSized
insidelower_poly_trait_ref
. The diagnostic is definitely worse, we now sayat least one trait is required for an object type
instead of`PointeeSized` cannot be used with trait objects
.Welp, I guess I should address the diagnostic regression (😩) since it only makes sense if you know the lowering… This kinda ties in with finding a better solution for #142693 (comment)