Do not try to reveal hidden types when trying to prove auto-traits in the defining scope #122192

oli-obk · 2024-03-08T12:41:56Z

this avoids the cycle error by just causing a selection error, which is not fatal. We pessimistically assume that freeze does not hold, which is always a safe assumption.

rustbot · 2024-03-08T12:42:04Z

r? @estebank

rustbot has assigned @estebank.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

rustbot · 2024-03-08T12:42:06Z

Some changes occurred to the core trait solver

cc @rust-lang/initiative-trait-system-refactor

compiler/rustc_errors/src/lib.rs

oli-obk · 2024-03-08T19:04:39Z

Reassigning reviewer because it allows more code on stable and needs trait system reviews

lcnr · 2024-03-11T14:22:34Z

happy with this change in general, unless this blocks the stabilization of ATPIT I would wait until #122077 is merged for an in-depth review

oli-obk · 2024-03-11T14:28:04Z

This PR is low priority and deserves some mir_const_qualifs cleanups and reduplications before landing anyway. The other two PRs this one is based on are needed for ATPIT

oli-obk · 2024-04-25T14:43:41Z

I would like to make more code compile by avoiding unnecessary (and false) query cycles in Freeze bound checking during const checks. Specifically, the following code snippet should compile (it doesn't before this PR):

const fn f() -> impl Eq {
    g()
}
const fn g() {}

The reason is that when checking whether the return type implements Freeze we end up trying to reveal the opaque type. Since we're also defining the opaque type, this causes a cycle. What this PR instead does (which is what the new solver already does), is to never reveal opaque types for auto traits if we're in the defining scope. Instead, we just bail out with a non-fatal selection error. This means we do not know the answer, but if the code asking cares only about knowing for sure a trait is implemented, but is fine pessimistically assuming it is not, then we can just do that. This means that even if the hidden type is Freeze, we assume it is not, which is always a safe decision, even if it means some code won't compile even though it could.

@rfcbot merge

rfcbot · 2024-04-25T14:43:43Z

Team member @oli-obk 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.

lcnr · 2024-04-25T14:59:41Z

As a quick note: this differs from the new solver in that the new solver instead normalizes the opaque type by looking at the opaque type storage, so the new solver may instead return ambiguity or success instead of NoSolution. The important part is that the new solver new gets query cycles here, so moving to the new solver will not cause any breakage.

Theoretically the incompleteness of the old solver may guide type inference, but as there are no trait implementations with Freeze bounds this should not happen. Even if it could, it would still be merely a theoretical concern.

@rfcbot reviewed

Rollup of 7 pull requests Successful merges: - rust-lang#122192 (Do not try to reveal hidden types when trying to prove Freeze in the defining scope) - rust-lang#124840 (resolve: mark it undetermined if single import is not has any bindings) - rust-lang#125622 (Winnow private method candidates instead of assuming any candidate of the right name will apply) - rust-lang#125871 (Orphanck[old solver]: Consider opaque types to never cover type parameters) - rust-lang#125893 (Handle all GVN binops in a single place.) - rust-lang#125911 (delete bootstrap build before switching to bumped rustc) - rust-lang#125918 (Revert: create const block bodies in typeck via query feeding) r? `@ghost` `@rustbot` modify labels: rollup

…ound_yay, r=<try> Do not use global cache for selection candidates if opaque types can be constrained fixes rust-lang#105787 r? `@ghost` This is certainly the crudest way to make the cache sound wrt opaque types, but if perf lets us get away with this, let's do it in the old solver and let the new solver fix this correctly once and for all. If perf is prohibitively bad, I'll look into alternatives (using canonical queries, checking whether any opaque types got constrained or whether decisions based on the availability of opaque types were made, still using the cache for things that can't possibly constrain opaque types (probably sound, famous last words), ..) cc rust-lang#122192 (comment) * [ ] check if this actually fixes rust-lang#105787 or if it just fixes the opaque type reproducer

bors · 2024-06-22T21:23:43Z

☔ The latest upstream changes (presumably #126838) made this pull request unmergeable. Please resolve the merge conflicts.

…ound_yay, r=lcnr Do not use global caches if opaque types can be defined fixes rust-lang#119272 r? `@lcnr` This is certainly a crude way to make the cache sound wrt opaque types, but since perf lets us get away with this, let's do it in the old solver and let the new solver fix this correctly once and for all. cc rust-lang#122192 (comment)

bors · 2024-07-24T15:01:51Z

☔ The latest upstream changes (presumably #126024) made this pull request unmergeable. Please resolve the merge conflicts.

…defining scope

…efining scope

oli-obk · 2024-07-24T16:01:31Z

@bors r=lcnr

bors · 2024-07-24T16:01:33Z

📌 Commit 8ea461d has been approved by lcnr

It is now in the queue for this repository.

…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#122192 (Do not try to reveal hidden types when trying to prove auto-traits in the defining scope) - rust-lang#126042 (Implement `unsigned_signed_diff`) - rust-lang#126548 (Improved clarity of documentation for std::fs::create_dir_all) - rust-lang#127717 (Fix malformed suggestion for repeated maybe unsized bounds) - rust-lang#128046 (Fix some `#[cfg_attr(not(doc), repr(..))]`) - rust-lang#128122 (Mark `missing_fragment_specifier` as `FutureReleaseErrorReportInDeps`) - rust-lang#128135 (std: use duplicate thread local state in tests) - rust-lang#128140 (Remove Unnecessary `.as_str()` Conversions) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#122192 - oli-obk:type_of_opaque_for_const_checks, r=lcnr Do not try to reveal hidden types when trying to prove auto-traits in the defining scope fixes rust-lang#99793 this avoids the cycle error by just causing a selection error, which is not fatal. We pessimistically assume that freeze does not hold, which is always a safe assumption.

rustbot assigned estebank Mar 8, 2024

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 Mar 8, 2024

oli-obk commented Mar 8, 2024

View reviewed changes

compiler/rustc_errors/src/lib.rs Outdated Show resolved Hide resolved

oli-obk added the T-lang Relevant to the language team, which will review and decide on the PR/issue. label Mar 8, 2024

oli-obk assigned lcnr and unassigned estebank Mar 8, 2024

lcnr mentioned this pull request Mar 11, 2024

recursive async function using Box::new results in cycle error #122332

Open

lcnr added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 11, 2024

traviscross mentioned this pull request Mar 25, 2024

Stabilize associated type position impl Trait (ATPIT) #120700

Open

3 tasks

oli-obk removed the S-blocked Status: Blocked on something else such as an RFC or other implementation work. label Apr 19, 2024

oli-obk force-pushed the type_of_opaque_for_const_checks branch from 69500f9 to 8de5565 Compare April 19, 2024 15:07

rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Apr 25, 2024

traviscross added A-const-fn A-impl-trait Area: `impl Trait`. Universally / existentially quantified anonymous types with static dispatch. labels May 23, 2024

apiraino removed the to-announce Announce this issue on triage meeting label Jun 7, 2024

oli-obk force-pushed the type_of_opaque_for_const_checks branch from ad6e85b to dd96364 Compare July 15, 2024 09:40

This comment has been minimized.

Sign in to view

oli-obk force-pushed the type_of_opaque_for_const_checks branch from dd96364 to 10930de Compare July 15, 2024 10:15

Add regression test

fdff100

oli-obk force-pushed the type_of_opaque_for_const_checks branch from 10930de to 232d370 Compare July 24, 2024 15:41

oli-obk added 3 commits July 24, 2024 16:00

Do not try to reveal hidden types when trying to prove Freeze in the …

acba644

…defining scope

Add regression tests

548c447

Do not assemble candidates for auto traits of opaque types in their d…

8ea461d

…efining scope

oli-obk force-pushed the type_of_opaque_for_const_checks branch from 232d370 to 8ea461d Compare July 24, 2024 16:00

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 24, 2024

matthiaskrgr mentioned this pull request Jul 24, 2024

Rollup of 8 pull requests #128155

Merged

bors merged commit 6bf5fd5 into rust-lang:master Jul 25, 2024
6 checks passed

rustbot added this to the 1.82.0 milestone Jul 25, 2024

lqd mentioned this pull request Oct 23, 2024

Revert #126024 "Do not use global caches if opaque types can be defined" #132075

Closed

RalfJung added the A-const-eval Area: Constant evaluation, covers all const contexts (static, const fn, ...) label Dec 1, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Do not try to reveal hidden types when trying to prove auto-traits in the defining scope #122192

Do not try to reveal hidden types when trying to prove auto-traits in the defining scope #122192

oli-obk commented Mar 8, 2024 •

edited

Loading

rustbot commented Mar 8, 2024

rustbot commented Mar 8, 2024

oli-obk commented Mar 8, 2024

lcnr commented Mar 11, 2024

oli-obk commented Mar 11, 2024

oli-obk commented Apr 25, 2024

rfcbot commented Apr 25, 2024 •

edited by spastorino

Loading

lcnr commented Apr 25, 2024

bors commented Jun 22, 2024

This comment has been minimized.

bors commented Jul 24, 2024

oli-obk commented Jul 24, 2024

bors commented Jul 24, 2024

Do not try to reveal hidden types when trying to prove auto-traits in the defining scope #122192

Do not try to reveal hidden types when trying to prove auto-traits in the defining scope #122192

Conversation

oli-obk commented Mar 8, 2024 • edited Loading

rustbot commented Mar 8, 2024

rustbot commented Mar 8, 2024

oli-obk commented Mar 8, 2024

lcnr commented Mar 11, 2024

oli-obk commented Mar 11, 2024

oli-obk commented Apr 25, 2024

rfcbot commented Apr 25, 2024 • edited by spastorino Loading

lcnr commented Apr 25, 2024

bors commented Jun 22, 2024

This comment has been minimized.

bors commented Jul 24, 2024

oli-obk commented Jul 24, 2024

bors commented Jul 24, 2024

oli-obk commented Mar 8, 2024 •

edited

Loading

rfcbot commented Apr 25, 2024 •

edited by spastorino

Loading