Skip to content

Rewrite relaxed bound policy code - #165677

Open
jdonszelmann wants to merge 1 commit into
rust-lang:mainfrom
jdonszelmann:relaxed-bounds-policies
Open

jdonszelmann wants to merge 1 commit into
rust-lang:mainfrom
jdonszelmann:relaxed-bounds-policies

Conversation

@jdonszelmann

@jdonszelmann jdonszelmann commented Nov 9, 2026 •

Copy link
Copy Markdown
Contributor

r? @WaffleLapkin

Because we may want to allow "relaxed bounds" more in the future as we land the Move and Forget traits, this code was such a mess it deserved a rewrite. There are some test changes, and I'll leave comments on each explaining why I think they're warranted. The old code is an absolute mess of logic, let me comment on it here:

        match rbp {
            // really means "all is allowed" which is dumb cause clearly some traits shouldn't be used
            // in relaxed bounds ever. However, if we say there are some places we don't error on because maybe duplicate errors
            // that's not consistent because below we will error on places we know for certain to have already errored on...
            RelaxedBoundPolicy::Allowed(dedup_map) => { 
                // `trait_def_id` only returns `None` for errors during resolution.
                let Some(trait_def_id) = trait_ref.trait_def_id() else { return };
                let tcx = self.tcx;
                let err = |s| {
                    let name = tcx.item_name(trait_def_id);
                    tcx.dcx()
                        .struct_span_err(
                            vec![span, s],
                            format!("duplicate relaxed `{name}` bounds"),
                        )
                        .with_code(E0203)
                        .emit();
                };
                dedup_map.entry(trait_def_id).and_modify(|&mut s| err(s)).or_insert(span);
                return;
            }
            // forbidden means "forbid all", though then we make some super subtle exceptions
            RelaxedBoundPolicy::Forbidden(reason) => {
                let gate = |context, subject| {
                    let extended = self.tcx.features().more_maybe_bounds();
                    let is_sized = trait_ref
                        .trait_def_id()
                        .is_some_and(|def_id| self.tcx.is_lang_item(def_id, LangItem::Sized));

                    // this code triggers also if there's a trait resolution error causing trait_def_id to return none,
                    // which is fine-ish except in other code paths we emit an error if that's the case
                    if extended && !is_sized {
                        return;
                    }

                    // we only talk about sized bounds if more_maybe_bounds is off, my new code always says "Sized bounds"
                    // which I think even without the feature improves the diagnostics
                    let prefix = if extended { "`Sized` " } else { "" };
                    let mut diag = self.dcx().struct_span_err(
                        span,
                        format!("relaxed {prefix}bounds are not permitted in {context}"),
                    );
                    if is_sized {
                        diag.note(format!(
                            "{subject} are not implicitly bounded by `Sized`, \
                             so there is nothing to relax"
                        ));
                    }
                    diag.emit();
                };

                match reason {
                    RelaxedBoundForbiddenReason::TraitObjectTy => {
                        gate("trait object types", "trait object types");
                        return;
                    }
                    RelaxedBoundForbiddenReason::SuperTrait => {
                        gate("supertrait bounds", "traits");
                        return;
                    }
                    RelaxedBoundForbiddenReason::TraitAlias => {
                        gate("trait alias bounds", "trait aliases");
                        return;
                    }
                    // In these cases we error regardless of whether trait_def_id is None or Some,
                    // and regardless of whether more_maybe_bounds is on. 
                    RelaxedBoundForbiddenReason::AssocTyBounds
                    | RelaxedBoundForbiddenReason::WhereBound => {}
                };
            }
        }

        self.dcx()
            .struct_span_err(span, "this relaxed bound is not permitted here")
            .with_note(
                "in this context, relaxed bounds are only allowed on \
                 type parameters defined on the closest item",
            )
            .emit();

Importantly, the code above intertwines the idea of Sized and feature(more_maybe_bounds) so much that it's absolutely impossible to distinguish more auto traits like Forget.

The new code:

        let deny = |trait_name: Option<&str>| {
              // always emits *some* diagnostic when we get here.
        };

        // `trait_def_id` only returns `None` for errors during resolution,
        // and there's no need to re-report those.
        let Some(trait_def_id) = trait_ref.trait_def_id() else { return };

        // if we see sized, allow it if we're in a position that sized is allowed to be relaxed in. Here we can add more traits like forget.
        if self.tcx.is_lang_item(trait_def_id, LangItem::Sized) {
            if !rbp.allowed_bounds.contains(AllowedRelaxedBounds::SIZED) {
                return deny(Some("Sized"));
            }
        // Like this we can explicitly disallow "OTHER" relaxed bounds
        // this code path only triggers when the feature is on
        } else if self.tcx.features().more_maybe_bounds()
            && !rbp.allowed_bounds.contains(AllowedRelaxedBounds::OTHER) {
            // This logic matches the old logic, even if I don't like it too much
            match rbp.reason {
                Some(RelaxedBoundForbiddenReason::TraitObjectTy)
                | Some(RelaxedBoundForbiddenReason::SuperTrait)
                | Some(RelaxedBoundForbiddenReason::TraitAlias)
                | None => {
                    return;
                }
                Some(RelaxedBoundForbiddenReason::AssocTyBounds)
                | Some(RelaxedBoundForbiddenReason::WhereBound)
                 => {
                    return deny(None)
                }
            }
        }
 
        // only once all checks pass, do we do a duplicate check and maybe allow it.
        let err = |s| {
            let name = tcx.item_name(trait_def_id);
            tcx.dcx()
                .struct_span_err(
                    vec![span, s],
                    format!("duplicate relaxed `{name}` bounds"),
                )
                .with_code(E0203)
                .emit();
        };
        rbp.dedup.entry(trait_def_id).and_modify(|&mut s| err(s)).or_insert(span);

@rustbot 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 Nov 9, 2026
@rustbot

rustbot commented Nov 9, 2026

Copy link
Copy Markdown
Collaborator

WaffleLapkin is currently at their maximum review capacity.
They may take a while to respond.

@rust-log-analyzer

This comment has been minimized.

@jdonszelmann
jdonszelmann force-pushed the relaxed-bounds-policies branch from a178158 to 11493ff Compare November 9, 2026 09:29
error: relaxed bounds are not permitted in trait object types
--> $DIR/missing-associated_item_or_field_def_ids.rs:3:29
|
LL | fn main() -> dyn Iterator + ?Iterator::advance_by(usize) {

@jdonszelmann jdonszelmann Nov 9, 2026 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

there's a nameres issue here, we stop emitting further errors now. If we don't do that it becomes really hard to check for specific traits that are allowed/disallowed in relaxed bounds.

View changes since the review

@@ -7,5 +7,4 @@ trait Tr {
fn main() {
let _: dyn Tr + ?Foo<Assoc = ()>;
//~^ ERROR: cannot find trait `Foo` in this scope
//~| ERROR: relaxed bounds are not permitted in trait object types

@jdonszelmann jdonszelmann Nov 9, 2026 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Same here, Foo has a nameres error

View changes since the review

@@ -4,14 +4,12 @@
#[lang = "default_trait2"] auto trait Trait2 {}

trait Trait3: ?Trait1 {}
//~^ ERROR relaxed bounds are not permitted in supertrait bounds
//~| ERROR bound modifier `?` can only be applied to `Sized`

@jdonszelmann jdonszelmann Nov 9, 2026 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

feature(more_maybe_bounds) is not on, and we already error in that case for non-?Sized relaxed bounds. This is just a duplicate.

View changes since the review

@@ -7,17 +7,17 @@ fn f<T: (Copy) + (?Sized) + (for<'a> Trait<'a>)>() {}

fn main() {
let _: Box<(Obj) + (?Sized) + (for<'a> Trait<'a>)>;
//~^ ERROR relaxed bounds are not permitted in trait object types

@jdonszelmann jdonszelmann Nov 9, 2026 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We start mentioning that it's a relaxed Sized bound specifically. I think it's a nicer error personally.

View changes since the review

@@ -6,21 +6,19 @@ use std::marker::{MetaSized, PointeeSized};
trait Sized_: Sized { }

trait NegSized: ?Sized { }
//~^ ERROR relaxed bounds are not permitted in supertrait bounds

@jdonszelmann jdonszelmann Nov 9, 2026 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@@ -3,7 +3,7 @@
// Ensure that relaxed bounds are not permitted in the `Self` bounds of trait aliases because trait
// aliases (like traits) aren't implicitly bounded by `Sized` so there's nothing to relax.

trait Alias0 = ?Sized; //~ ERROR relaxed bounds are not permitted in trait alias bounds

@jdonszelmann jdonszelmann Nov 9, 2026 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@@ -12,8 +12,7 @@ struct S3<T>(T) where &'static T: ?Sized; //~ ERROR this relaxed bound is not pe
trait Trait<'a> {}

struct S4<T>(T) where for<'a> T: ?Trait<'a>;
//~^ ERROR this relaxed bound is not permitted here

@jdonszelmann jdonszelmann Nov 9, 2026 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

feature(more_maybe_bounds) is not on, and we already error in that case for non-?Sized relaxed bounds. This is just a duplicate.

View changes since the review

#[derive(Copy, Clone, Debug)]
struct AllowedRelaxedBounds: u32 {
const SIZED = 1 << 0;
const FORGET = 1 << 1;

@jdonszelmann jdonszelmann Nov 9, 2026 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I could remove Forget here again. It's temporarily unused, though will soon be. I guess I kept it as an example of how this will work. Doesn't do much harm either.

View changes since the review

Comment thread compiler/rustc_ast_lowering/src/lib.rs Outdated
span,
format!(
"relaxed {}bounds are not permitted in {context}",
trait_name.map(|i| format!("{i} ")).unwrap_or_default()

@fmease fmease Nov 9, 2026 •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
trait_name.map(|i| format!("{i} ")).unwrap_or_default()
trait_name.map(|i| format!("`{i}` ")).unwrap_or_default()

View changes since the review

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

good catch, changed it!

@jdonszelmann
jdonszelmann force-pushed the relaxed-bounds-policies branch from 11493ff to e14043c Compare November 10, 2026 09:54
@rustbot

rustbot commented Nov 10, 2026

Copy link
Copy Markdown
Collaborator

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@rust-bors

rust-bors Bot commented Nov 10, 2026

Copy link
Copy Markdown
Contributor

☔ The latest upstream changes (presumably #165760) made this pull request unmergeable. Please resolve the merge conflicts by rebasing.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants