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.

@jdonszelmann

Copy link
Copy Markdown
Contributor Author

r? @JonathanBrouwer

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.

6 participants