Skip to content

Conversation

@adwinwhite
Copy link
Contributor

@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. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver) labels Oct 30, 2025
Copy link
Contributor

@lcnr lcnr left a comment

Choose a reason for hiding this comment

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

Trait objects always have their associated types specified so candidates won't be empty.

That comments exists to state why we never treat the alias as rigid in this case?

CAn you add something along the lines of

"This is somewhat inconsistent and may make #57893 slightly easier to exploit. However, it matches the behavior of the old solver"

as a comment

View changes since this review

@@ -0,0 +1,17 @@
//@ compile-flags: -Znext-solver
//@ check-pass

Copy link
Contributor

Choose a reason for hiding this comment

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

pls add // Regression test for trait-system-refactor-initiative#whatever

// See `tests/ui/winnowing/norm-where-bound-gt-alias-bound.rs`.
if candidates.iter().any(|c| matches!(c.source, CandidateSource::ParamEnv(_))) {
candidates.retain(|c| matches!(c.source, CandidateSource::ParamEnv(_)));
} else if matches!(goal.predicate.self_ty().kind(), ty::Dynamic(..)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should move this into assemble_and_evaluate_candidates as rn this doesn't handle cases where the self type is an alias, e.g. the following should still fail with ur PR

trait Trait {
    type Assoc;
}
trait Id {
    type This;
}
impl<T: ?Sized> Id for T {
    type This = T;
}

fn foo<T>(x: <dyn Trait<Assoc = T> as Trait>::Assoc) -> T
where
    <dyn Trait<Assoc = T> as Id>::This: Trait,
{
    x
}

Copy link
Contributor Author

@adwinwhite adwinwhite Oct 31, 2025

Choose a reason for hiding this comment

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

This doesn't fail (after fixing ?Sized bound). And the Assoc projection's self_ty is not alias?
The trait goal in param env has alias self_ty but it's irrelevant for the NormalizesTo goal.

Besides, the following doesn't fail either. Perhaps the self_ty is normalized somehow?

trait Trait {
    type Assoc;
}
trait Id {
    type This: ?Sized;
}
impl<T: ?Sized> Id for T {
    type This = T;
}

fn foo<T>(x: <<dyn Trait<Assoc = T> as Id>::This as Trait>::Assoc) -> T
where
    <dyn Trait<Assoc = T> as Id>::This: Trait,
{
    x
}

Nonetheless, I moved the code into assemble_and_evaluate_candidates and removed comments specific to NormalizesTo goal since trait goals also use this method.

Copy link
Contributor

@lcnr lcnr Nov 3, 2025

Choose a reason for hiding this comment

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

you're right. I messed up the example.

does the following fail with the old impl? I think the issue is that we otherwise first normalize the inner, and then the outer alias 🤔

trait Trait {
    type Assoc;
}
trait Id<'a> {
    type This: ?Sized;
}
impl<T: ?Sized> Id<'_> for T {
    type This = T;
}

fn foo<T>(x: for<'a> fn(
    <<dyn Trait<Assoc = T> as Id<'a>>::This as Trait>::Assoc
)) -> fn(T)
where
    dyn Trait<Assoc = T>: Trait,
{
    x
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This doesn't fail with my first commit.

It turns out when we add NormalizesTo goal for projection Assoc, we also add a nested AliasRelate goal for the inner projection This.
And we evaluate the inner projection before the outer one.
Thus when we evaluate the NormalizesTo goal of Assoc, the self_ty is already normalized.

Copy link
Contributor

Choose a reason for hiding this comment

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

ah yeah. we never actually need to normalize the self type for normalization goals 😅 can you still add that one as a test?

@adwinwhite
Copy link
Contributor Author

That comments exists to state why we never treat the alias as rigid in this case?

Yes, just a reminder that we won't take the inject_normalize_to_rigid_candidate code path in this case.

@lcnr
Copy link
Contributor

lcnr commented Nov 5, 2025

r=me after adding the additional test

@adwinwhite
Copy link
Contributor Author

🤔, I don't think I have the permission to "r=you".

@lcnr
Copy link
Contributor

lcnr commented Nov 6, 2025

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Nov 6, 2025

📌 Commit d2cfc47 has been approved by lcnr

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 6, 2025
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Nov 6, 2025
…te, r=lcnr

Un-shadow object bound candidate in `NormalizesTo` goal if self_ty is trait object

Fixes rust-lang/trait-system-refactor-initiative#244

r? lcnr
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Nov 6, 2025
…te, r=lcnr

Un-shadow object bound candidate in `NormalizesTo` goal if self_ty is trait object

Fixes rust-lang/trait-system-refactor-initiative#244

r? lcnr
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Nov 6, 2025
…te, r=lcnr

Un-shadow object bound candidate in `NormalizesTo` goal if self_ty is trait object

Fixes rust-lang/trait-system-refactor-initiative#244

r? lcnr
bors added a commit that referenced this pull request Nov 6, 2025
Rollup of 7 pull requests

Successful merges:

 - #146861 (add extend_front to VecDeque with specialization like extend)
 - #148213 (Fix invalid tag closing when leaving expansion "original code")
 - #148292 (Un-shadow object bound candidate in `NormalizesTo` goal if self_ty is trait object)
 - #148528 (run-make tests: use edition 2024)
 - #148554 (Add regression test for issue 148542)
 - #148561 (Fix ICE from async closure variance)
 - #148563 (rustdoc-search: remove broken index special case)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit fc31898 into rust-lang:master Nov 6, 2025
11 checks passed
@rustbot rustbot added this to the 1.93.0 milestone Nov 6, 2025
rust-timer added a commit that referenced this pull request Nov 6, 2025
Rollup merge of #148292 - adwinwhite:assemble_object_candidate, r=lcnr

Un-shadow object bound candidate in `NormalizesTo` goal if self_ty is trait object

Fixes rust-lang/trait-system-refactor-initiative#244

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

Labels

S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

vortex-array: param-env shadowing of object candidates

4 participants