Skip to content

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

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 20 additions & 7 deletions compiler/rustc_hir_analysis/src/hir_ty_lowering/bounds.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,17 +85,17 @@ fn search_bounds_for<'tcx>(
}
}

fn collect_unbounds<'tcx>(
fn collect_relaxed_bounds<'tcx>(
hir_bounds: &'tcx [hir::GenericBound<'tcx>],
self_ty_where_predicates: Option<(LocalDefId, &'tcx [hir::WherePredicate<'tcx>])>,
) -> SmallVec<[&'tcx PolyTraitRef<'tcx>; 1]> {
let mut unbounds: SmallVec<[_; 1]> = SmallVec::new();
let mut relaxed_bounds: SmallVec<[_; 1]> = SmallVec::new();
search_bounds_for(hir_bounds, self_ty_where_predicates, |ptr| {
if matches!(ptr.modifiers.polarity, hir::BoundPolarity::Maybe(_)) {
unbounds.push(ptr);
relaxed_bounds.push(ptr);
}
});
unbounds
relaxed_bounds
}

fn collect_bounds<'a, 'tcx>(
Expand Down Expand Up @@ -209,9 +209,22 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
return;
}
} else {
// Report invalid unbounds on sizedness-bounded generic parameters.
let unbounds = collect_unbounds(hir_bounds, self_ty_where_predicates);
self.check_and_report_invalid_unbounds_on_param(unbounds);
// Report invalid relaxed bounds.
// FIXME: Since we only call this validation function here in this function, we only
// fully validate relaxed bounds in contexts where we perform
// "sized elaboration". In most cases that doesn't matter because we *usually*
// reject relaxed bounds outright during AST lowering.
// However, this can easily get out of sync! Ideally, we would perform this step
// where we are guaranteed to catch *all* bounds like in
// `Self::lower_poly_trait_ref`. List of concrete issues:
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(We likely can't actually do it in lower_poly_trait_ref because we need access to all the neighboring HIR bounds)

// FIXME(more_maybe_bounds): We don't call this for e.g., trait object tys or
// supertrait bounds!
// FIXME(trait_alias, #143122): We don't call it for the RHS. Arguably however,
// AST lowering should reject them outright.
// FIXME(associated_type_bounds): We don't call this for them. However, AST
// lowering should reject them outright (#135229).
let bounds = collect_relaxed_bounds(hir_bounds, self_ty_where_predicates);
self.check_and_report_invalid_relaxed_bounds(bounds);
}

let collected = collect_sizedness_bounds(tcx, hir_bounds, self_ty_where_predicates, span);
Expand Down
47 changes: 22 additions & 25 deletions compiler/rustc_hir_analysis/src/hir_ty_lowering/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use rustc_errors::{
};
use rustc_hir::def::{CtorOf, DefKind, Res};
use rustc_hir::def_id::DefId;
use rustc_hir::{self as hir, HirId, LangItem, PolyTraitRef};
use rustc_hir::{self as hir, HirId, PolyTraitRef};
use rustc_middle::bug;
use rustc_middle::ty::fast_reject::{TreatParams, simplify_type};
use rustc_middle::ty::print::{PrintPolyTraitRefExt as _, PrintTraitRefExt as _};
Expand All @@ -35,26 +35,24 @@ use crate::fluent_generated as fluent;
use crate::hir_ty_lowering::{AssocItemQSelf, HirTyLowerer};

impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
/// Check for multiple relaxed default bounds and relaxed bounds of non-sizedness traits.
pub(crate) fn check_and_report_invalid_unbounds_on_param(
/// Check for duplicate relaxed bounds and relaxed bounds of non-default traits.
pub(crate) fn check_and_report_invalid_relaxed_bounds(
&self,
unbounds: SmallVec<[&PolyTraitRef<'_>; 1]>,
relaxed_bounds: SmallVec<[&PolyTraitRef<'_>; 1]>,
) {
let tcx = self.tcx();

let sized_did = tcx.require_lang_item(LangItem::Sized, DUMMY_SP);

let mut unique_bounds = FxIndexSet::default();
let mut seen_repeat = false;
for unbound in &unbounds {
if let Res::Def(DefKind::Trait, unbound_def_id) = unbound.trait_ref.path.res {
seen_repeat |= !unique_bounds.insert(unbound_def_id);
for bound in &relaxed_bounds {
if let Res::Def(DefKind::Trait, trait_def_id) = bound.trait_ref.path.res {
seen_repeat |= !unique_bounds.insert(trait_def_id);
}
}

if unbounds.len() > 1 {
if relaxed_bounds.len() > 1 {
let err = errors::MultipleRelaxedDefaultBounds {
spans: unbounds.iter().map(|ptr| ptr.span).collect(),
spans: relaxed_bounds.iter().map(|ptr| ptr.span).collect(),
};

if seen_repeat {
Expand All @@ -64,24 +62,23 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
};
}

for unbound in unbounds {
if let Res::Def(DefKind::Trait, did) = unbound.trait_ref.path.res
&& ((did == sized_did) || tcx.is_default_trait(did))
let sized_def_id = tcx.require_lang_item(hir::LangItem::Sized, DUMMY_SP);

for bound in relaxed_bounds {
if let Res::Def(DefKind::Trait, def_id) = bound.trait_ref.path.res
&& (def_id == sized_def_id || tcx.is_default_trait(def_id))
{
continue;
}

let unbound_traits = match tcx.sess.opts.unstable_opts.experimental_default_bounds {
true => "`?Sized` and `experimental_default_bounds`",
false => "`?Sized`",
};
self.dcx().span_err(
unbound.span,
format!(
"relaxing a default bound only does something for {}; all other traits are \
not bound by default",
unbound_traits
),
bound.span,
if tcx.sess.opts.unstable_opts.experimental_default_bounds
|| tcx.features().more_maybe_bounds()
{
"bound modifier `?` can only be applied to default traits like `Sized`"
} else {
"bound modifier `?` can only be applied to `Sized`"
},
);
}
}
Expand Down
8 changes: 4 additions & 4 deletions tests/ui/feature-gates/feature-gate-more-maybe-bounds.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,14 @@ fn foo(_: Box<dyn Trait1 + ?Trait2>) {}
//~^ ERROR relaxed bounds are not permitted in trait object types
fn bar<T: ?Trait1 + ?Trait2>(_: T) {}
//~^ ERROR type parameter has more than one relaxed default bound, only one is supported
//~| ERROR relaxing a default bound only does something for `?Sized`; all other traits are not bound by default
//~| ERROR relaxing a default bound only does something for `?Sized`; all other traits are not bound by default
//~| ERROR bound modifier `?` can only be applied to `Sized`
//~| ERROR bound modifier `?` can only be applied to `Sized`

trait Trait {}
// Do not suggest `#![feature(more_maybe_bounds)]` for repetitions
fn baz<T: ?Trait + ?Trait>(_ : T) {}
//~^ ERROR type parameter has more than one relaxed default bound, only one is supported
//~| ERROR relaxing a default bound only does something for `?Sized`; all other traits are not bound by default
//~| ERROR relaxing a default bound only does something for `?Sized`; all other traits are not bound by default
//~| ERROR bound modifier `?` can only be applied to `Sized`
//~| ERROR bound modifier `?` can only be applied to `Sized`

fn main() {}
8 changes: 4 additions & 4 deletions tests/ui/feature-gates/feature-gate-more-maybe-bounds.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,13 @@ LL | fn bar<T: ?Trait1 + ?Trait2>(_: T) {}
= help: add `#![feature(more_maybe_bounds)]` 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: relaxing a default bound only does something for `?Sized`; all other traits are not bound by default
error: bound modifier `?` can only be applied to `Sized`
--> $DIR/feature-gate-more-maybe-bounds.rs:10:11
|
LL | fn bar<T: ?Trait1 + ?Trait2>(_: T) {}
| ^^^^^^^

error: relaxing a default bound only does something for `?Sized`; all other traits are not bound by default
error: bound modifier `?` can only be applied to `Sized`
--> $DIR/feature-gate-more-maybe-bounds.rs:10:21
|
LL | fn bar<T: ?Trait1 + ?Trait2>(_: T) {}
Expand All @@ -53,13 +53,13 @@ error[E0203]: type parameter has more than one relaxed default bound, only one i
LL | fn baz<T: ?Trait + ?Trait>(_ : T) {}
| ^^^^^^ ^^^^^^

error: relaxing a default bound only does something for `?Sized`; all other traits are not bound by default
error: bound modifier `?` can only be applied to `Sized`
--> $DIR/feature-gate-more-maybe-bounds.rs:17:11
|
LL | fn baz<T: ?Trait + ?Trait>(_ : T) {}
| ^^^^^^

error: relaxing a default bound only does something for `?Sized`; all other traits are not bound by default
error: bound modifier `?` can only be applied to `Sized`
--> $DIR/feature-gate-more-maybe-bounds.rs:17:20
|
LL | fn baz<T: ?Trait + ?Trait>(_ : T) {}
Expand Down
4 changes: 2 additions & 2 deletions tests/ui/impl-trait/opt-out-bound-not-satisfied.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@

use std::future::Future;
fn foo() -> impl ?Future<Output = impl Send> {
//~^ ERROR: relaxing a default bound only does something for `?Sized`
//~| ERROR: relaxing a default bound only does something for `?Sized`
//~^ ERROR: bound modifier `?` can only be applied to `Sized`
//~| ERROR: bound modifier `?` can only be applied to `Sized`
()
}

Expand Down
4 changes: 2 additions & 2 deletions tests/ui/impl-trait/opt-out-bound-not-satisfied.stderr
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
error: relaxing a default bound only does something for `?Sized`; all other traits are not bound by default
error: bound modifier `?` can only be applied to `Sized`
--> $DIR/opt-out-bound-not-satisfied.rs:5:18
|
LL | fn foo() -> impl ?Future<Output = impl Send> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: relaxing a default bound only does something for `?Sized`; all other traits are not bound by default
error: bound modifier `?` can only be applied to `Sized`
--> $DIR/opt-out-bound-not-satisfied.rs:5:18
|
LL | fn foo() -> impl ?Future<Output = impl Send> {
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/issues/issue-37534.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
struct Foo<T: ?Hash> {}
//~^ ERROR expected trait, found derive macro `Hash`
//~| ERROR relaxing a default bound only does something for `?Sized`
//~| ERROR bound modifier `?` can only be applied to `Sized`

fn main() {}
2 changes: 1 addition & 1 deletion tests/ui/issues/issue-37534.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ help: consider importing this trait instead
LL + use std::hash::Hash;
|

error: relaxing a default bound only does something for `?Sized`; all other traits are not bound by default
error: bound modifier `?` can only be applied to `Sized`
--> $DIR/issue-37534.rs:1:15
|
LL | struct Foo<T: ?Hash> {}
Expand Down
8 changes: 4 additions & 4 deletions tests/ui/issues/issue-87199.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,12 @@

// Check that these function definitions only emit warnings, not errors
fn arg<T: ?Send>(_: T) {}
//~^ ERROR: relaxing a default bound only does something for `?Sized`
//~^ ERROR: bound modifier `?` can only be applied to `Sized`
fn ref_arg<T: ?Send>(_: &T) {}
//~^ ERROR: relaxing a default bound only does something for `?Sized`
//~^ ERROR: bound modifier `?` can only be applied to `Sized`
fn ret() -> impl Iterator<Item = ()> + ?Send { std::iter::empty() }
//~^ ERROR: relaxing a default bound only does something for `?Sized`
//~| ERROR: relaxing a default bound only does something for `?Sized`
//~^ ERROR: bound modifier `?` can only be applied to `Sized`
//~| ERROR: bound modifier `?` can only be applied to `Sized`

// Check that there's no `?Sized` relaxation!
fn main() {
Expand Down
8 changes: 4 additions & 4 deletions tests/ui/issues/issue-87199.stderr
Original file line number Diff line number Diff line change
@@ -1,22 +1,22 @@
error: relaxing a default bound only does something for `?Sized`; all other traits are not bound by default
error: bound modifier `?` can only be applied to `Sized`
--> $DIR/issue-87199.rs:8:11
|
LL | fn arg<T: ?Send>(_: T) {}
| ^^^^^

error: relaxing a default bound only does something for `?Sized`; all other traits are not bound by default
error: bound modifier `?` can only be applied to `Sized`
--> $DIR/issue-87199.rs:10:15
|
LL | fn ref_arg<T: ?Send>(_: &T) {}
| ^^^^^

error: relaxing a default bound only does something for `?Sized`; all other traits are not bound by default
error: bound modifier `?` can only be applied to `Sized`
--> $DIR/issue-87199.rs:12:40
|
LL | fn ret() -> impl Iterator<Item = ()> + ?Send { std::iter::empty() }
| ^^^^^

error: relaxing a default bound only does something for `?Sized`; all other traits are not bound by default
error: bound modifier `?` can only be applied to `Sized`
--> $DIR/issue-87199.rs:12:40
|
LL | fn ret() -> impl Iterator<Item = ()> + ?Send { std::iter::empty() }
Expand Down
4 changes: 2 additions & 2 deletions tests/ui/sized-hierarchy/default-bound.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,13 @@ fn neg_sized<T: ?Sized>() {}
fn metasized<T: MetaSized>() {}

fn neg_metasized<T: ?MetaSized>() {}
//~^ ERROR relaxing a default bound only does something for `?Sized`; all other traits are not bound by default
//~^ ERROR bound modifier `?` can only be applied to `Sized`


fn pointeesized<T: PointeeSized>() { }

fn neg_pointeesized<T: ?PointeeSized>() { }
//~^ ERROR relaxing a default bound only does something for `?Sized`; all other traits are not bound by default
//~^ ERROR bound modifier `?` can only be applied to `Sized`


fn main() {
Expand Down
4 changes: 2 additions & 2 deletions tests/ui/sized-hierarchy/default-bound.stderr
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
error: relaxing a default bound only does something for `?Sized`; all other traits are not bound by default
error: bound modifier `?` can only be applied to `Sized`
--> $DIR/default-bound.rs:16:21
|
LL | fn neg_metasized<T: ?MetaSized>() {}
| ^^^^^^^^^^

error: relaxing a default bound only does something for `?Sized`; all other traits are not bound by default
error: bound modifier `?` can only be applied to `Sized`
--> $DIR/default-bound.rs:22:24
|
LL | fn neg_pointeesized<T: ?PointeeSized>() { }
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/trait-bounds/maybe-bound-has-path-args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@ trait Trait {}

fn test<T: ?self::<i32>::Trait>() {}
//~^ ERROR type arguments are not allowed on module `maybe_bound_has_path_args`
//~| ERROR relaxing a default bound only does something for `?Sized`
//~| ERROR bound modifier `?` can only be applied to `Sized`

fn main() {}
2 changes: 1 addition & 1 deletion tests/ui/trait-bounds/maybe-bound-has-path-args.stderr
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
error: relaxing a default bound only does something for `?Sized`; all other traits are not bound by default
error: bound modifier `?` can only be applied to `Sized`
--> $DIR/maybe-bound-has-path-args.rs:3:12
|
LL | fn test<T: ?self::<i32>::Trait>() {}
Expand Down
4 changes: 2 additions & 2 deletions tests/ui/trait-bounds/maybe-bound-with-assoc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@ trait HasAssoc {
type Assoc;
}
fn hasassoc<T: ?HasAssoc<Assoc = ()>>() {}
//~^ ERROR relaxing a default bound
//~^ ERROR bound modifier `?` can only be applied to `Sized`

trait NoAssoc {}
fn noassoc<T: ?NoAssoc<Missing = ()>>() {}
//~^ ERROR relaxing a default bound
//~^ ERROR bound modifier `?` can only be applied to `Sized`
//~| ERROR associated type `Missing` not found for `NoAssoc`

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

error: relaxing a default bound only does something for `?Sized`; all other traits are not bound by default
error: bound modifier `?` can only be applied to `Sized`
--> $DIR/maybe-bound-with-assoc.rs:8:15
|
LL | fn noassoc<T: ?NoAssoc<Missing = ()>>() {}
Expand Down
29 changes: 29 additions & 0 deletions tests/ui/trait-bounds/more_maybe_bounds.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// FIXME(more_maybe_bounds): Even under `more_maybe_bounds` / `-Zexperimental-default-bounds`,
// trying to relax non-default bounds should still be an error in all contexts! As you can see
// there are placed like supertrait bounds and trait object types where we currently don't perform
// this check.
#![feature(auto_traits, more_maybe_bounds, negative_impls)]

trait Trait1 {}
auto trait Trait2 {}

// FIXME: `?Trait1` should be rejected, `Trait1` isn't marked `#[lang = "default_traitN"]`.
trait Trait3: ?Trait1 {}
trait Trait4 where Self: Trait1 {}

// FIXME: `?Trait2` should be rejected, `Trait2` isn't marked `#[lang = "default_traitN"]`.
fn foo(_: Box<(dyn Trait3 + ?Trait2)>) {}
fn bar<T: ?Sized + ?Trait2 + ?Trait1 + ?Trait4>(_: &T) {}
//~^ ERROR bound modifier `?` can only be applied to default traits like `Sized`
//~| ERROR bound modifier `?` can only be applied to default traits like `Sized`
//~| ERROR bound modifier `?` can only be applied to default traits like `Sized`

struct S;
impl !Trait2 for S {}
impl Trait1 for S {}
impl Trait3 for S {}

fn main() {
foo(Box::new(S));
bar(&S);
}
20 changes: 20 additions & 0 deletions tests/ui/trait-bounds/more_maybe_bounds.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
error: bound modifier `?` can only be applied to default traits like `Sized`
--> $DIR/more_maybe_bounds.rs:16:20
|
LL | fn bar<T: ?Sized + ?Trait2 + ?Trait1 + ?Trait4>(_: &T) {}
| ^^^^^^^

error: bound modifier `?` can only be applied to default traits like `Sized`
--> $DIR/more_maybe_bounds.rs:16:30
|
LL | fn bar<T: ?Sized + ?Trait2 + ?Trait1 + ?Trait4>(_: &T) {}
| ^^^^^^^

error: bound modifier `?` can only be applied to default traits like `Sized`
--> $DIR/more_maybe_bounds.rs:16:40
|
LL | fn bar<T: ?Sized + ?Trait2 + ?Trait1 + ?Trait4>(_: &T) {}
| ^^^^^^^

error: aborting due to 3 previous errors

Loading