-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Fix trait solver overflow with non_local_definitions
lint
#123594
Conversation
I think I minimized a test: pub trait Test {}
impl<'a, T: 'a> Test for &[T] where &'a T: Test {}
fn main() {
struct Local {}
impl Test for &Local {}
} |
ec3ee99
to
5250336
Compare
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
…w, r=<try> Fix trait solver overflow with `non_local_definitions` lint This PR fixes the trait solver overflow with the `non_local_definitions` lint reported in rust-lang#123573 using the suggestion from `@lcnr:` rust-lang#123573 (comment) to use the next trait solver. ~~I have not (yet) tried to create a minimized repro~~ ``@compiler-errors`` did the minimization (thanks you) but I have manually tested on the `starlark-rust` project that it fixes the issue. Fixes rust-lang#123573 r? `@lcnr`
☀️ Try build successful - checks-actions |
This comment has been minimized.
This comment has been minimized.
Finished benchmarking commit (fff3bb1): comparison URL. Overall result: ❌ regressions - no action neededBenchmarking 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 Instruction countThis is a highly reliable metric that was used to determine the overall result at the top of this comment.
Max RSS (memory usage)This benchmark run did not return any relevant results for this metric. CyclesThis benchmark run did not return any relevant results for this metric. Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 667.477s -> 665.775s (-0.25%) |
cc @rust-lang/types. this PR changes a lint to always use the new trait solver by default to avoid errors from fatal overflow with the old solver. This lint always runs in an empty environment and does not use generic params. It therefore does not rely on any additional behavior not required for |
Going to start a types FCP to officially state that going forward, the @rust-lang/initiative-trait-system-refactor has the authority to enable the use of the new solver on stable without FCPs as long as t-types is pinged in the PR and the affected area is neither relevant for soundness nor backwards compatibility. @rfcbot fcp merge |
Team member @lcnr has proposed to merge this. The next step is review by the rest of the tagged team members: No concerns currently listed. Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! See this document for info about what commands tagged team members can give me. |
…=compiler-errors Split `non_local_definitions` lint tests in separate test files This PR splits the giant `non_local_definitions` lint UI test in separate test files. This change is extracted from rust-lang#123594 (where it was requested rust-lang#123594 (comment)), to ease the review of the other PR and to reduce the size of the other PR. r? `@compiler-errors`
…=compiler-errors Split `non_local_definitions` lint tests in separate test files This PR splits the giant `non_local_definitions` lint UI test in separate test files. This change is extracted from rust-lang#123594 (where it was requested rust-lang#123594 (comment)), to ease the review of the other PR and to reduce the size of the other PR. r? ``@compiler-errors``
Rollup merge of rust-lang#123653 - Urgau:split-test-non_local_defs, r=compiler-errors Split `non_local_definitions` lint tests in separate test files This PR splits the giant `non_local_definitions` lint UI test in separate test files. This change is extracted from rust-lang#123594 (where it was requested rust-lang#123594 (comment)), to ease the review of the other PR and to reduce the size of the other PR. r? ``@compiler-errors``
☔ The latest upstream changes (presumably #123676) made this pull request unmergeable. Please resolve the merge conflicts. |
1286592
to
c2e2245
Compare
🔔 This is now entering its final comment period, as per the review above. 🔔 |
The final comment period, with a disposition to merge, as per the review above, is now complete. As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed. This will be merged soon. |
@bors r+ |
☀️ Test successful - checks-actions |
Finished benchmarking commit (f22a0c2): comparison URL. Overall result: ❌ regressions - no action needed@rustbot label: -perf-regression Instruction countThis is a highly reliable metric that was used to determine the overall result at the top of this comment.
Max RSS (memory usage)This benchmark run did not return any relevant results for this metric. CyclesThis benchmark run did not return any relevant results for this metric. Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 671.849s -> 669.682s (-0.32%) |
This PR fixes the trait solver overflow with the
non_local_definitions
lint reported in #123573 using the suggestion from @lcnr: #123573 (comment) to use the next trait solver.I have not (yet) tried to create a minimized repro@compiler-errors
did the minimization (thanks you) but I have manually tested on thestarlark-rust
project that it fixes the issue.Fixes #123573
r? @lcnr