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

split NormalizesTo out of Projection #112658

Closed
wants to merge 2 commits into from

Conversation

lcnr
Copy link
Contributor

@lcnr lcnr commented Jun 15, 2023

@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. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver) labels Jun 15, 2023
@@ -112,6 +110,9 @@ pub(super) trait GoalKind<'tcx>:
// Consider a clause, which consists of a "assumption" and some "requirements",
// to satisfy a goal. If the requirements hold, then attempt to satisfy our
// goal by equating it with the assumption.
//
// FIXME: change this function to take an actual `Clause` as assumption instead
// of a predicate.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think we should land this without first separately handling that FIXME.

With NormalizesTo as a goal getting proven by Projection in the environment it's otherwise far too easy to accidentally put a NormalizesTo into the environment which is useless. Even without bugs from this, it is still very confusing.

@rust-log-analyzer

This comment has been minimized.

Err(NoSolution)
}
let alias = goal.predicate.projection_ty.to_ty(self.tcx()).into();
self.eq(goal.param_env, alias, goal.predicate.term)?;
Copy link
Member

@compiler-errors compiler-errors Jun 15, 2023

Choose a reason for hiding this comment

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

should we skip all the nonsense and just emit an alias-eq here directly?

Well, actually, I guess this does behave differently on alias projection ?0 now 🤔

@lcnr lcnr force-pushed the normalizes-to-projection-split branch from ce1d66b to 5f71900 Compare June 16, 2023 08:47
@rust-log-analyzer
Copy link
Collaborator

The job mingw-check-tidy failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
Prepare all required actions
Getting action download info
Download action repository 'actions/checkout@v3' (SHA:c85c95e3d7251135ab7dc9ce3241c5835cc595a9)
Download action repository 'rust-lang/simpleinfra@master' (SHA:5aa3313339663c490d58f9face5433f8c68a95e9)
Complete job name: PR - mingw-check-tidy
git config --global core.autocrlf false
shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
env:
  CI_JOB_NAME: mingw-check-tidy
---
Building wheels for collected packages: reuse
  Building wheel for reuse (pyproject.toml): started
  Building wheel for reuse (pyproject.toml): finished with status 'done'
  Created wheel for reuse: filename=reuse-1.1.0-cp310-cp310-manylinux_2_35_x86_64.whl size=180116 sha256=351235b2326fb4db7a18e257e13ce7896c5f77339521e2c2612e71e154800a19
  Stored in directory: /tmp/pip-ephem-wheel-cache-dirzaslk/wheels/c2/3c/b9/1120c2ab4bd82694f7e6f0537dc5b9a085c13e2c69a8d0c76d
Installing collected packages: boolean-py, binaryornot, setuptools, reuse, python-debian, markupsafe, license-expression, jinja2, chardet
  Attempting uninstall: setuptools
    Found existing installation: setuptools 59.6.0
    Not uninstalling setuptools at /usr/lib/python3/dist-packages, outside environment /usr
---
Successfully tagged rust-ci:latest
Built container sha256:795c3a29b0298b144e7eb5987af1b30c0e8607103301f14ff5bec06bd23392c6
Uploading finished image to https://ci-caches.rust-lang.org/docker/e51156f19850ce886cec818e46dc2a021e0aa7c270d15673e8fe74cd8522fc8ac3995109aebb688ee49ed586735ed4cf5f8c06d44c48298fa09c35ae4b082281

<botocore.awsrequest.AWSRequest object at 0x7f664a204350>
gzip: stdout: Broken pipe
xargs: docker: terminated by signal 13
[CI_JOB_NAME=mingw-check-tidy]
[CI_JOB_NAME=mingw-check-tidy]
---
   Compiling tidy v0.1.0 (/checkout/src/tools/tidy)
    Finished release [optimized] target(s) in 16.24s
##[endgroup]
fmt check
Diff in /checkout/compiler/rustc_trait_selection/src/solve/normalizes_to.rs at line 416:
             ty::Binder::dummy(ty::ProjectionPredicate {
                 projection_ty: ecx.tcx().mk_alias_ty(goal.predicate.def_id(), [self_ty]),
                 term,
-            }).to_predicate(tcx),
+            .to_predicate(tcx),
+            .to_predicate(tcx),
             // Technically, we need to check that the future type is Sized,
             // but that's already proven by the generator being WF.
             [],
Diff in /checkout/compiler/rustc_trait_selection/src/solve/normalizes_to.rs at line 457:
                     .tcx()
                     .mk_alias_ty(goal.predicate.def_id(), [self_ty, generator.resume_ty()]),
                 term,
-            }).to_predicate(tcx),
+            .to_predicate(tcx),
+            .to_predicate(tcx),
             // Technically, we need to check that the future type is Sized,
             // but that's already proven by the generator being WF.
             [],
Running `"/checkout/obj/build/x86_64-unknown-linux-gnu/rustfmt/bin/rustfmt" "--config-path" "/checkout" "--edition" "2021" "--unstable-features" "--skip-children" "--check" "/checkout/compiler/rustc_trait_selection/src/traits/specialize/specialization_graph.rs" "/checkout/compiler/rustc_trait_selection/src/traits/object_safety.rs" "/checkout/compiler/rustc_trait_selection/src/solve/alias_relate.rs" "/checkout/compiler/rustc_trait_selection/src/traits/util.rs" "/checkout/compiler/rustc_trait_selection/src/solve/normalizes_to.rs" "/checkout/compiler/rustc_trait_selection/src/traits/select/mod.rs" "/checkout/compiler/rustc_trait_selection/src/traits/error_reporting/ambiguity.rs" "/checkout/compiler/rustc_trait_selection/src/traits/specialize/mod.rs"` failed.
If you're running `tidy`, try again with `--bless`. Or, if you just want to format code, run `./x.py fmt` instead.

@lcnr
Copy link
Contributor Author

lcnr commented Jun 16, 2023

with this the following example currently fails:

fn foo(i: isize) -> isize { i + 1 }

fn apply<A, F>(f: F, v: A) where F: FnOnce(A) -> A { f(v); }

pub fn main() {
    apply(foo, 2)
}

the reason is that we try to prove Projection(<?0 as FnOnce(?1)>::Assoc, ?1) which equates <?0 as FnOnce(?1)>::Assoc with ?1 which incorrectly fails the occurs check.

I think that to fix this we can either change the generalizer to emit NormalizesTo goals in that case, going to do that in a separate PR.

@jackh726
Copy link
Member

Huh, that's exactly the problem I ran into with Chalk :)

@bors
Copy link
Contributor

bors commented Jun 17, 2023

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

@compiler-errors
Copy link
Member

@rustbot author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 29, 2023
@lcnr lcnr closed this Sep 29, 2023
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 8, 2023
…3, r=BoxyUwU

split `NormalizesTo` out of `Projection` 3

third attempt at rust-lang#112658. Rebasing rust-lang#116262 is very annoying, so I am doing it again from scratch. We should now be able to merge it without regressing anything as we handle occurs check failures involving aliases correctly since rust-lang#117088.

see https://hackmd.io/ktEL8knTSYmtdfrMMnA-Hg

fixes rust-lang/trait-system-refactor-initiative#1

r? `@compiler-errors`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc 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.

normalizes-to bound in old solver is stronger than new solver
6 participants