Skip to content

Eagerly decide whether relaxed bounds are allowed or not#157305

Draft
oli-obk wants to merge 2 commits into
rust-lang:mainfrom
oli-obk:relaxed-bounds-maybe
Draft

Eagerly decide whether relaxed bounds are allowed or not#157305
oli-obk wants to merge 2 commits into
rust-lang:mainfrom
oli-obk:relaxed-bounds-maybe

Conversation

@oli-obk
Copy link
Copy Markdown
Contributor

@oli-obk oli-obk commented Jun 2, 2026

r? @davidtwco or @fmease

Previously introduced in #142693 by @fmease

I am trying to resolve

// FIXME: Since we only call this validation function here in this function, we only
, but then got into a rabbit hole somehwere.

I think this PR further unblocks resolving that FIXME, because it makes it easier to reason about whether we should be reporting an error about duplicate bounds or about relaxed bounds in general

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 2, 2026
@oli-obk
Copy link
Copy Markdown
Contributor Author

oli-obk commented Jun 2, 2026

@bors try @rust-timer queue

probably not an issue, but let's see

@rust-timer

This comment has been minimized.

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jun 2, 2026
@rust-bors

This comment has been minimized.

rust-bors Bot pushed a commit that referenced this pull request Jun 2, 2026
Eagerly decide whether relaxed bounds are allowed or not
@fmease
Copy link
Copy Markdown
Member

fmease commented Jun 2, 2026

Allowing (T): ?Sized needs T-lang buy-in strictly speaking.

(Personally I'd even be fine with allowing duplicate relaxed bounds, they're not harmful; not my call though, of course)

As my FIXME that you're trying to address alludes to, ideally we'd move all the relaxed bounds logic from AST lowering to HIR ty lowering where "sized elab" actually happens but for that we'd need to pass some new context / policy parameter to lower_poly_trait_ref. I don't know if that's feasible.

@fmease
Copy link
Copy Markdown
Member

fmease commented Jun 2, 2026

I think this PR further unblocks resolving that FIXME, because it makes it easier to reason about whether we should be reporting an error about duplicate bounds or about relaxed bounds in general

I assume you plan on moving that flawed HIR ty lowering to AST lowering? I guess that's a valid approach, right.

Comment thread compiler/rustc_ast_lowering/src/item.rs Outdated
{
RelaxedBoundPolicy::Allowed
} else {
RelaxedBoundPolicy::Forbidden(RelaxedBoundForbiddenReason::LateBoundVarsInScope)
Copy link
Copy Markdown
Member

@fmease fmease Jun 2, 2026

Choose a reason for hiding this comment

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

LateBoundVarsInScope is not the right reason for relaxed bounds on parent type parameters. Could you add a separate reason or generalize (rename) this one (I know that the diagnostic ends up looking the same)?

View changes since the review

@rust-bors
Copy link
Copy Markdown
Contributor

rust-bors Bot commented Jun 2, 2026

☀️ Try build successful (CI)
Build commit: 19e0554 (19e05544551e3be0f27a16d6ba430efa865ca875, parent: 4f84d9fac456d973d592cf3fb48db958ecf22506)

@rust-timer

This comment has been minimized.

@rust-timer
Copy link
Copy Markdown
Collaborator

Finished benchmarking commit (19e0554): comparison URL.

Overall result: ❌ regressions - no action needed

Benchmarking means the PR may be perf-sensitive. Consider adding rollup=never if this change is not fit for rolling up.

@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

mean range count
Regressions ❌
(primary)
0.1% [0.1%, 0.2%] 2
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 0.1% [0.1%, 0.2%] 2

Max RSS (memory usage)

Results (primary 3.9%, secondary 1.5%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
3.9% [3.4%, 4.3%] 2
Regressions ❌
(secondary)
5.3% [5.3%, 5.3%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-2.3% [-2.3%, -2.3%] 1
All ❌✅ (primary) 3.9% [3.4%, 4.3%] 2

Cycles

Results (primary 1.8%, secondary 2.1%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
1.8% [1.8%, 1.8%] 1
Regressions ❌
(secondary)
2.1% [2.1%, 2.1%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 1.8% [1.8%, 1.8%] 1

Binary size

This perf run didn't have relevant results for this metric.

Bootstrap: 510.625s -> 510.015s (-0.12%)
Artifact size: 400.75 MiB -> 400.71 MiB (-0.01%)

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jun 2, 2026
@oli-obk oli-obk force-pushed the relaxed-bounds-maybe branch from 1b39cd5 to 876d5f3 Compare June 2, 2026 15:24
@oli-obk
Copy link
Copy Markdown
Contributor Author

oli-obk commented Jun 2, 2026

Allowing (T): ?Sized needs T-lang buy-in strictly speaking.

I reverted that

I assume you plan on moving that flawed HIR ty lowering to AST lowering? I guess that's a valid approach, right.

jup

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

Labels

S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. 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