search

rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
93.93k stars 12.09k forks source link

cleanup `candidate_should_be_dropped_in_favor_of` #124592

Open lcnr opened 1 month ago

lcnr commented 1 month ago

and make sure it's actually a partial ordering :< the previous ordering was:

The new solver currently does not implement the lowered priority of global ParamEnv candidates, so the divergence between it and the old solver in the added tests will be resolved in favor of the old solver behavior after this PR.

r? @compiler-errors

rust-log-analyzer commented 1 month ago

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) ```plain Getting action download info Download action repository 'msys2/setup-msys2@v2.22.0' (SHA:cc11e9188b693c2b100158c3322424c4cc1dadea) Download action repository 'actions/checkout@v4' (SHA:0ad4b8fadaa221de15dcec353f45205ec38ea70b) Download action repository 'actions/upload-artifact@v4' (SHA:65462800fd760344b1a7b4382951275a0abb4808) Complete job name: PR - mingw-check-tidy git config --global core.autocrlf false shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0} --- COPY scripts/sccache.sh /scripts/ RUN sh /scripts/sccache.sh COPY host-x86_64/mingw-check/reuse-requirements.txt /tmp/ RUN pip3 install --no-deps --no-cache-dir --require-hashes -r /tmp/reuse-requirements.txt \ && pip3 install virtualenv COPY host-x86_64/mingw-check/validate-toolstate.sh /scripts/ COPY host-x86_64/mingw-check/validate-error-codes.sh /scripts/ # NOTE: intentionally uses python2 for x.py so we can test it still works. # NOTE: intentionally uses python2 for x.py so we can test it still works. # validate-toolstate only runs in our CI, so it's ok for it to only support python3. ENV SCRIPT TIDY_PRINT_DIFF=1 python2.7 ../x.py test \ --stage 0 src/tools/tidy tidyselftest --extra-checks=py:lint # This file is autogenerated by pip-compile with Python 3.10 # by the following command: # # pip-compile --allow-unsafe --generate-hashes reuse-requirements.in --- #12 [5/8] COPY host-x86_64/mingw-check/reuse-requirements.txt /tmp/ #12 DONE 0.0s #13 [6/8] RUN pip3 install --no-deps --no-cache-dir --require-hashes -r /tmp/reuse-requirements.txt && pip3 install virtualenv #13 0.432 Downloading binaryornot-0.4.4-py2.py3-none-any.whl (9.0 kB) #13 0.450 Collecting boolean-py==4.0 #13 0.457 Downloading boolean.py-4.0-py3-none-any.whl (25 kB) #13 0.473 Collecting chardet==5.1.0 --- #13 3.626 Building wheels for collected packages: reuse #13 3.627 Building wheel for reuse (pyproject.toml): started #13 3.948 Building wheel for reuse (pyproject.toml): finished with status 'done' #13 3.949 Created wheel for reuse: filename=reuse-1.1.0-cp310-cp310-manylinux_2_35_x86_64.whl size=181117 sha256=f5f58750481f69515c2c0d1d503daf565e2565c370d07fc6aeb95fe3498b4269 #13 3.949 Stored in directory: /tmp/pip-ephem-wheel-cache-v8e9u12q/wheels/c2/3c/b9/1120c2ab4bd82694f7e6f0537dc5b9a085c13e2c69a8d0c76d #13 3.952 Installing collected packages: boolean-py, binaryornot, setuptools, reuse, python-debian, markupsafe, license-expression, jinja2, chardet #13 3.973 Attempting uninstall: setuptools #13 3.974 Found existing installation: setuptools 59.6.0 #13 3.975 Not uninstalling setuptools at /usr/lib/python3/dist-packages, outside environment /usr #13 3.975 Not uninstalling setuptools at /usr/lib/python3/dist-packages, outside environment /usr #13 3.976 Can't uninstall 'setuptools'. No files were found to uninstall. #13 4.631 Successfully installed binaryornot-0.4.4 boolean-py-4.0 chardet-5.1.0 jinja2-3.1.2 license-expression-30.0.0 markupsafe-2.1.1 python-debian-0.1.49 reuse-1.1.0 setuptools-66.0.0 #13 4.631 WARNING: Running pip as the 'root' user can result in broken permissions and conflicting behaviour with the system package manager. It is recommended to use a virtual environment instead: https://pip.pypa.io/warnings/venv #13 5.152 Collecting virtualenv #13 5.203 Downloading virtualenv-20.26.1-py3-none-any.whl (3.9 MB) #13 5.330 ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 3.9/3.9 MB 31.2 MB/s eta 0:00:00 #13 5.381 Collecting platformdirs<5,>=3.9.1 #13 5.388 Downloading platformdirs-4.2.1-py3-none-any.whl (17 kB) #13 5.407 Collecting distlib<1,>=0.3.7 #13 5.414 Downloading distlib-0.3.8-py2.py3-none-any.whl (468 kB) #13 5.458 Collecting filelock<4,>=3.12.2 #13 5.465 Downloading filelock-3.14.0-py3-none-any.whl (12 kB) #13 5.465 Downloading filelock-3.14.0-py3-none-any.whl (12 kB) #13 5.549 Installing collected packages: distlib, platformdirs, filelock, virtualenv #13 5.710 Successfully installed distlib-0.3.8 filelock-3.14.0 platformdirs-4.2.1 virtualenv-20.26.1 #13 DONE 5.8s #14 [7/8] COPY host-x86_64/mingw-check/validate-toolstate.sh /scripts/ #14 DONE 0.0s --- DirectMap4k: 208832 kB DirectMap2M: 7131136 kB DirectMap1G: 11534336 kB ##[endgroup] Executing TIDY_PRINT_DIFF=1 python2.7 ../x.py test --stage 0 src/tools/tidy tidyselftest --extra-checks=py:lint + TIDY_PRINT_DIFF=1 python2.7 ../x.py test --stage 0 src/tools/tidy tidyselftest --extra-checks=py:lint Finished `dev` profile [unoptimized] target(s) in 0.03s ##[endgroup] downloading https://ci-artifacts.rust-lang.org/rustc-builds-alt/9ba3d315b4b2bea7485071eaf4b2681320cd2a27/rust-dev-nightly-x86_64-unknown-linux-gnu.tar.xz extracting /checkout/obj/build/cache/llvm-9ba3d315b4b2bea7485071eaf4b2681320cd2a27-true/rust-dev-nightly-x86_64-unknown-linux-gnu.tar.xz to /checkout/obj/build/x86_64-unknown-linux-gnu/ci-llvm --- Finished `release` profile [optimized] target(s) in 24.35s ##[endgroup] fmt check tidy check tidy error: file `/checkout/tests/ui/traits/issue-66768.rs` no longer exists and should be removed from the exclusions in `src/tools/tidy/src/issues.txt` removing old virtual environment removing old virtual environment creating virtual environment at '/checkout/obj/build/venv' using 'python3.10' Requirement already satisfied: pip in ./build/venv/lib/python3.10/site-packages (24.0) Collecting black==23.3.0 (from -r /checkout/src/tools/tidy/config/requirements.txt (line 7)) ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 1.7/1.7 MB 17.2 MB/s eta 0:00:00 Collecting click==8.1.3 (from -r /checkout/src/tools/tidy/config/requirements.txt (line 34)) Downloading click-8.1.3-py3-none-any.whl (96 kB) ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 96.6/96.6 kB 34.9 MB/s eta 0:00:00 ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 96.6/96.6 kB 34.9 MB/s eta 0:00:00 Collecting importlib-metadata==6.7.0 (from -r /checkout/src/tools/tidy/config/requirements.txt (line 38)) Downloading importlib_metadata-6.7.0-py3-none-any.whl (22 kB) Collecting mypy-extensions==1.0.0 (from -r /checkout/src/tools/tidy/config/requirements.txt (line 42)) Downloading mypy_extensions-1.0.0-py3-none-any.whl (4.7 kB) Collecting packaging==23.1 (from -r /checkout/src/tools/tidy/config/requirements.txt (line 46)) Downloading packaging-23.1-py3-none-any.whl (48 kB) Collecting pathspec==0.11.1 (from -r /checkout/src/tools/tidy/config/requirements.txt (line 50)) Downloading pathspec-0.11.1-py3-none-any.whl (29 kB) Collecting platformdirs==3.6.0 (from -r /checkout/src/tools/tidy/config/requirements.txt (line 54)) Downloading platformdirs-3.6.0-py3-none-any.whl (16 kB) Downloading platformdirs-3.6.0-py3-none-any.whl (16 kB) Collecting ruff==0.0.272 (from -r /checkout/src/tools/tidy/config/requirements.txt (line 58)) Downloading ruff-0.0.272-py3-none-manylinux_2_17_x86_64.manylinux2014_x86_64.whl (5.9 MB) Collecting tomli==2.0.1 (from -r /checkout/src/tools/tidy/config/requirements.txt (line 77)) Downloading tomli-2.0.1-py3-none-any.whl (12 kB) Collecting typed-ast==1.5.4 (from -r /checkout/src/tools/tidy/config/requirements.txt (line 81)) Downloading typed_ast-1.5.4-cp310-cp310-manylinux_2_5_x86_64.manylinux1_x86_64.manylinux_2_12_x86_64.manylinux2010_x86_64.whl (877 kB) Downloading typed_ast-1.5.4-cp310-cp310-manylinux_2_5_x86_64.manylinux1_x86_64.manylinux_2_12_x86_64.manylinux2010_x86_64.whl (877 kB) ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 877.7/877.7 kB 68.2 MB/s eta 0:00:00 Collecting typing-extensions==4.6.3 (from -r /checkout/src/tools/tidy/config/requirements.txt (line 107)) Downloading typing_extensions-4.6.3-py3-none-any.whl (31 kB) Collecting zipp==3.15.0 (from -r /checkout/src/tools/tidy/config/requirements.txt (line 114)) Downloading zipp-3.15.0-py3-none-any.whl (6.8 kB) Installing collected packages: zipp, typing-extensions, typed-ast, tomli, ruff, platformdirs, pathspec, packaging, mypy-extensions, click, importlib-metadata, black Successfully installed black-23.3.0 click-8.1.3 importlib-metadata-6.7.0 mypy-extensions-1.0.0 packaging-23.1 pathspec-0.11.1 platformdirs-3.6.0 ruff-0.0.272 tomli-2.0.1 typed-ast-1.5.4 typing-extensions-4.6.3 zipp-3.15.0 some tidy checks failed Build completed unsuccessfully in 0:00:59 local time: Wed May 1 18:35:30 UTC 2024 network time: Wed, 01 May 2024 18:35:30 GMT ```
lcnr commented 1 month ago

@bors try

lcnr commented 1 month ago

@bors try

bors commented 1 month ago

:hourglass: Trying commit a0359c8fddeca7ffe8c75277e1da71cb94ab83a6 with merge 2466b92ed5feb282a98bb984f45d1e6d39f8c1aa...

bors commented 1 month ago

:sunny: Try build successful - checks-actions Build commit: 2466b92ed5feb282a98bb984f45d1e6d39f8c1aa (2466b92ed5feb282a98bb984f45d1e6d39f8c1aa)

compiler-errors commented 1 month ago

@craterbot check

craterbot commented 1 month ago

:ok_hand: Experiment pr-124592 created and queued. :robot: Automatically detected try build 2466b92ed5feb282a98bb984f45d1e6d39f8c1aa :mag: You can check out the queue and this experiment's details.

:information_source: Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

craterbot commented 4 weeks ago

:construction: Experiment pr-124592 is now running

:information_source: Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

craterbot commented 3 weeks ago

:tada: Experiment pr-124592 is completed! :bar_chart: 51 regressed and 4 fixed (444840 total) :newspaper: Open the full report.

:warning: If you notice any spurious failure please add them to the blacklist! :information_source: Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

fmease commented 3 weeks ago

:bar_chart: 51 regressed and 4 fixed (444840 total)

Minimizations:

  1. tc-collection
    • Minimized (playground)
    • Error message of MIN differs slightly cmp'ed to ORIG but that's not substantial
      • We still have a u64 / T mismatch
  2. Andlon/ice_testcase
    • Already tracked in tests/ui/associated-item/issue-105449.rs
  3. droundy/sad-monte-carlo
  4. imgui
  5. 47 “unknown” build failures
    • TODO(fmease)
fmease commented 2 weeks ago

@lcnr, I just went through the crater reports of all 47 “unknown” regressions. Some of them were spurious (no space left on device), all of the the remaining ones are type mismatches of the expected kind as well as secondary regressions only (except for redst4r/bustools_cli-rs / bustools_cli-0.2.1) with the primary regression being imgui in most cases and tc-collection otherwise (downstream crates are owned by the same person (monorepo) in the latter case).

As discussed, this is ready for an FCP. I will open “incoming breakage” PRs for the relevant crates if/once this PR gets accepted.

lcnr commented 2 weeks ago

This PR changes the rule to candidate winnowing during selection. We prefer candidates over others, both to guide inference and to avoid ambiguity in case there are multipe applicable candidates.

The current preference rules aren't a proper order, making candidate selection order-dependent. The issue arrises for "global ParamEnv candidates": where-bounds which are not higher-ranked and do not mention any generic parameters.

This current preference rules are as follows

non-global ParamEnv > impl
impl > global ParamEnv
global ParamEnv == non-global ParamEnv

I propose to change these rules to a proper ordering:

non-global ParamEnv > impl > global ParamEnv

This avoids order-dependence and is easier to support in the next-generation trait solver. The current behavior is also unintuitive.

This change adds additional incomplete inference guidance in cases with

With this setup, we previously did not use the impls to drop the global ParamEnv candidate as the impls were first dropped via the non-global candidate.

If there were more than 2 or 3 impl candidates, we ended dropping the global ParamEnv candidates via an impl candidate before all impls were dropped due to the non-global ParamEnv candidate. This subtle behavior is caused by using swap_remove when dropping impl candidates: https://github.com/rust-lang/rust/blob/cf2baaa8350c9cac9a5bc139926bcb246303b9f8/compiler/rustc_trait_selection/src/traits/select/mod.rs#L498-L508

This causes the following patterns to change behavior

while keeping the behavior of the patterns in

The new solver currently does not implement the lowered priority of global ParamEnv candidates, so the divergence between it and the old solver in the added tests will be resolved in favor of the old solver behavior after this PR.

There are multiple existing crates which break due to inference changes, see https://github.com/rust-lang/rust/pull/124592#issuecomment-2104541495. All breakage should be fixable by adding explicit type annotations and we intend to open PRs doing so once this is in FCP. Thank you @fmease for minimizing them.

@rfcbot fcp merge

rfcbot commented 2 weeks ago

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.

BoxyUwU commented 1 week ago

is imgui not a relatively significant crate to be breaking? I would expect that to be being used by a lot of projects :thinking:

lcnr commented 1 week ago

I still think we should go ahead with it. The fix to it will be local. I don't mind if we have to keep this regression on nightly for 1 or 2 versions