Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check generic argument compatibility when projecting assoc ty #102488

Merged
merged 1 commit into from
Oct 4, 2022

Conversation

compiler-errors
Copy link
Member

Fixes #102114

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Sep 29, 2022
@rust-highfive
Copy link
Collaborator

r? @oli-obk

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 29, 2022
@compiler-errors
Copy link
Member Author

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion.

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

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Sep 29, 2022
@bors
Copy link
Contributor

bors commented Sep 29, 2022

⌛ Trying commit ee713f3 with merge 09ce9c766f77b0361214b3db82b0b1d18e258ca3...

Comment on lines +2178 to +2181
if let Some(parent) = generics.parent
&& let parent_generics = tcx.generics_of(parent)
&& !check_substs_compatible_inner(tcx, parent_generics, parent_args) {
return false;
Copy link
Member

Choose a reason for hiding this comment

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

TIL githubs highlighting does not support let-chains 😆

@bors
Copy link
Contributor

bors commented Sep 29, 2022

☀️ Try build successful - checks-actions
Build commit: 09ce9c766f77b0361214b3db82b0b1d18e258ca3 (09ce9c766f77b0361214b3db82b0b1d18e258ca3)

@rust-timer
Copy link
Collaborator

Queued 09ce9c766f77b0361214b3db82b0b1d18e258ca3 with parent 9f1a21a, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (09ce9c766f77b0361214b3db82b0b1d18e258ca3): comparison URL.

Overall result: no relevant changes - no action needed

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean1 range count2
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
2.3% [2.3%, 2.3%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-3.3% [-3.3%, -3.3%] 1
All ❌✅ (primary) - - 0

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean1 range count2
Regressions ❌
(primary)
3.7% [3.7%, 3.7%] 1
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 3.7% [3.7%, 3.7%] 1

Footnotes

  1. the arithmetic mean of the percent change 2

  2. number of relevant changes 2

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Sep 30, 2022
@oli-obk
Copy link
Contributor

oli-obk commented Oct 4, 2022

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Oct 4, 2022

📌 Commit ee713f3 has been approved by oli-obk

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 Oct 4, 2022
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 4, 2022
…iaskrgr

Rollup of 6 pull requests

Successful merges:

 - rust-lang#102241 (Package `rust-docs-json` into nightly components (take 3))
 - rust-lang#102488 (Check generic argument compatibility when projecting assoc ty)
 - rust-lang#102647 (Only allow ~const bounds for traits with #[const_trait])
 - rust-lang#102648 (Add test for rust-lang#102605)
 - rust-lang#102651 (It's not about types or consts, but the lack of regions)
 - rust-lang#102653 (resolve instance: missing value to `delay_span_bug`)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 5d58451 into rust-lang:master Oct 4, 2022
@rustbot rustbot added this to the 1.66.0 milestone Oct 4, 2022
@compiler-errors compiler-errors deleted the gat-compatibility branch November 2, 2022 02:56
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jul 25, 2024
Make sure that args are compatible in `resolve_associated_item`

Implements a similar check to the one that we have in projection for GATs (rust-lang#102488, rust-lang#123240), where we check that the args of an impl item are compatible before returning it. This is done in `resolve_assoc_item`, which is backing `Instance::resolve`, so this is conceptually generalizing the check from GATs to methods/assoc consts. This is important to make sure that the inliner will only visit and substitute MIR bodies that are compatible w/ their trait definitions.

This shouldn't happen in codegen, but there are a few ways to get the inliner to be invoked (via calls to `optimized_mir`) before codegen, namely polymorphization and CTFE.

Fixes rust-lang#121957
Fixes rust-lang#120792
Fixes rust-lang#120793
Fixes rust-lang#121063
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jul 25, 2024
Rollup merge of rust-lang#128171 - compiler-errors:arg-compat, r=oli-obk

Make sure that args are compatible in `resolve_associated_item`

Implements a similar check to the one that we have in projection for GATs (rust-lang#102488, rust-lang#123240), where we check that the args of an impl item are compatible before returning it. This is done in `resolve_assoc_item`, which is backing `Instance::resolve`, so this is conceptually generalizing the check from GATs to methods/assoc consts. This is important to make sure that the inliner will only visit and substitute MIR bodies that are compatible w/ their trait definitions.

This shouldn't happen in codegen, but there are a few ways to get the inliner to be invoked (via calls to `optimized_mir`) before codegen, namely polymorphization and CTFE.

Fixes rust-lang#121957
Fixes rust-lang#120792
Fixes rust-lang#120793
Fixes rust-lang#121063
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

expected type for b/#0 (b/0) but found Lifetime(ReStatic) when substituting, substs=[ReStatic]
7 participants