Use a post-monomorphization typing env when mangling components that come from impls #135149
+99
−38
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rustbot
added
S-waiting-on-review
compiler-errors
commented
Jan 6, 2025
| impl_trait_ref.map(|impl_trait_ref| impl_trait_ref.instantiate(self.tcx, args)), | ||
| ) | ||
| } else { | ||
| // We are probably printing a nested item inside of an impl. |
There was a problem hiding this comment.
This happens when we try to mangle a nested function inside of an impl, like:
impl<T> Foo for T {
fn outer() {
fn inner() {}
}
}
When mangling inner, we try to print it like <T as Foo>::outer::inner, however since inner doesn't inherit the substitutions from outer, we give it fresh identity substs.
I'm kinda of the opinion that we should print outer item def paths as plain old crate::{impl#0}::outer::inner rather than printing with the trait ref. I think that would break things tho 🤷
| let mut typing_env = ty::TypingEnv::post_analysis(self.tcx, impl_def_id); | ||
| if !args.is_empty() { | ||
| typing_env.param_env = | ||
| ty::EarlyBinder::bind(typing_env.param_env).instantiate(self.tcx, args); |
There was a problem hiding this comment.
This was the problematic substutiton.
|
@bors r+ |
bors
added
S-waiting-on-bors
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this pull request
Jan 7, 2025
Use a post-monomorphization typing env when mangling components that come from impls When mangling associated methods of impls, we were previously using the wrong param-env. Instead of using a fully monomorphized param-env like we usually do in codegen, we were taking the post-analysis param-env, and treating it as an early binder to *re-substitute* the impl args. I've pointed out the problematic old code in an inline comment. This would give us param-envs with possibly trivial predicates that would prevent normalization via param-env shadowing. In the example test linked below, `tests/ui/symbol-names/normalize-in-param-env.rs`, this happens when we mangle the impl `impl<P: Point2> MyFrom<P::S> for P` with the substitution `P = Vec2`. Because the where clause of the impl is `P: Point2`, which elaborates to `[P: Point2, P: Point, <P as Point>::S projects-to <P as Point2>::S2]` and the fact that `impl Point2 for Vec2` normalizes `Vec2::S2` to `Vec2::S`, this causes a cycle. The proper fix here is to use a fully monomorphized param-env for the case where the impl is properly substituted. Fixes rust-lang#135143 While rust-lang#134081 uncovered this bug for legacy symbol mangling, it was preexisting for v0 symbol mangling. This PR fixes both. The test requires a "hack" because we strip the args of the instance we're printing for legacy symbol mangling except for drop glue, so we box a closure to ensure we generate drop glue. r? oli-obk
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Jan 7, 2025
…iaskrgr Rollup of 7 pull requests Successful merges: - rust-lang#131146 (Stop clearing box's drop flags early) - rust-lang#133810 (remove unnecessary `eval_verify_bound`) - rust-lang#134745 (Normalize each signature input/output in `typeck_with_fallback` with its own span) - rust-lang#134989 (Lower Guard Patterns to HIR.) - rust-lang#135149 (Use a post-monomorphization typing env when mangling components that come from impls) - rust-lang#135171 (rustdoc: use stable paths as preferred canonical paths) - rust-lang#135200 (rustfmt: drop nightly-gating of the `--style-edition` flag registration) r? `@ghost` `@rustbot` modify labels: rollup
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Jan 7, 2025
…iaskrgr Rollup of 6 pull requests Successful merges: - rust-lang#133810 (remove unnecessary `eval_verify_bound`) - rust-lang#134745 (Normalize each signature input/output in `typeck_with_fallback` with its own span) - rust-lang#134989 (Lower Guard Patterns to HIR.) - rust-lang#135149 (Use a post-monomorphization typing env when mangling components that come from impls) - rust-lang#135171 (rustdoc: use stable paths as preferred canonical paths) - rust-lang#135200 (rustfmt: drop nightly-gating of the `--style-edition` flag registration) r? `@ghost` `@rustbot` modify labels: rollup
rust-timer
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Jan 8, 2025
Rollup merge of rust-lang#135149 - compiler-errors:mangle, r=oli-obk Use a post-monomorphization typing env when mangling components that come from impls When mangling associated methods of impls, we were previously using the wrong param-env. Instead of using a fully monomorphized param-env like we usually do in codegen, we were taking the post-analysis param-env, and treating it as an early binder to *re-substitute* the impl args. I've pointed out the problematic old code in an inline comment. This would give us param-envs with possibly trivial predicates that would prevent normalization via param-env shadowing. In the example test linked below, `tests/ui/symbol-names/normalize-in-param-env.rs`, this happens when we mangle the impl `impl<P: Point2> MyFrom<P::S> for P` with the substitution `P = Vec2`. Because the where clause of the impl is `P: Point2`, which elaborates to `[P: Point2, P: Point, <P as Point>::S projects-to <P as Point2>::S2]` and the fact that `impl Point2 for Vec2` normalizes `Vec2::S2` to `Vec2::S`, this causes a cycle. The proper fix here is to use a fully monomorphized param-env for the case where the impl is properly substituted. Fixes rust-lang#135143 While rust-lang#134081 uncovered this bug for legacy symbol mangling, it was preexisting for v0 symbol mangling. This PR fixes both. The test requires a "hack" because we strip the args of the instance we're printing for legacy symbol mangling except for drop glue, so we box a closure to ensure we generate drop glue. r? oli-obk
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When mangling associated methods of impls, we were previously using the wrong param-env. Instead of using a fully monomorphized param-env like we usually do in codegen, we were taking the post-analysis param-env, and treating it as an early binder to re-substitute the impl args. I've pointed out the problematic old code in an inline comment.
This would give us param-envs with possibly trivial predicates that would prevent normalization via param-env shadowing.
In the example test linked below,
tests/ui/symbol-names/normalize-in-param-env.rs, this happens when we mangle the implimpl<P: Point2> MyFrom<P::S> for Pwith the substitutionP = Vec2. Because the where clause of the impl isP: Point2, which elaborates to[P: Point2, P: Point, <P as Point>::S projects-to <P as Point2>::S2]and the fact thatimpl Point2 for Vec2normalizesVec2::S2toVec2::S, this causes a cycle.The proper fix here is to use a fully monomorphized param-env for the case where the impl is properly substituted.
Fixes #135143
While #134081 uncovered this bug for legacy symbol mangling, it was preexisting for v0 symbol mangling. This PR fixes both. The test requires a "hack" because we strip the args of the instance we're printing for legacy symbol mangling except for drop glue, so we box a closure to ensure we generate drop glue.
r? oli-obk