Skip to content

gate let_chains in all places before the 2024 edition #140198

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

fee1-dead
Copy link
Member

Since let_chains relies on the 2024 edition drop order to be consistent, it makes sense for us to gate it in all places, even when there are unstable features such as with if_let_guards in matches. This removes the need for us to pass in an enum to CondChecker as we can just always pass in the edition instead.

cc @est31

Since `let_chains` relies on the 2024 edition drop order to be
consistent, it makes sense for us to gate it in all places, even when
there are unstable features such as with `if_let_guard`s in matches.
This removes the need for us to pass in an enum to `CondChecker` as we
can just always pass in the edition instead.
@rustbot
Copy link
Collaborator

rustbot commented Apr 23, 2025

r? @wesleywiser

rustbot has assigned @wesleywiser.
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 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. labels Apr 23, 2025
@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)
info: removing rustup binaries
info: rustup is uninstalled
##[group]Image checksum input
mingw-check-tidy
# We use the ghcr base image because ghcr doesn't have a rate limit
# and the mingw-check-tidy job doesn't cache docker images in CI.
FROM ghcr.io/rust-lang/ubuntu:22.04

ARG DEBIAN_FRONTEND=noninteractive
RUN apt-get update && apt-get install -y --no-install-recommends \
  g++ \
  make \
---

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.
# 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,cpp
#
# 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 2.873 Building wheels for collected packages: reuse
#12 2.874   Building wheel for reuse (pyproject.toml): started
#12 3.086   Building wheel for reuse (pyproject.toml): finished with status 'done'
#12 3.087   Created wheel for reuse: filename=reuse-4.0.3-cp310-cp310-manylinux_2_35_x86_64.whl size=132719 sha256=5bb60f62728aaedff7162745ce743c7f2f55069b3e7f82e6a37d70df455797cc
#12 3.087   Stored in directory: /tmp/pip-ephem-wheel-cache-29sg4hn8/wheels/3d/8d/0a/e0fc6aba4494b28a967ab5eaf951c121d9c677958714e34532
#12 3.089 Successfully built reuse
#12 3.090 Installing collected packages: boolean-py, binaryornot, tomlkit, reuse, python-debian, markupsafe, license-expression, jinja2, chardet, attrs
#12 3.486 Successfully installed attrs-23.2.0 binaryornot-0.4.4 boolean-py-4.0 chardet-5.2.0 jinja2-3.1.4 license-expression-30.3.0 markupsafe-2.1.5 python-debian-0.1.49 reuse-4.0.3 tomlkit-0.13.0
#12 3.486 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
#12 4.025 Collecting virtualenv
#12 4.092   Downloading virtualenv-20.30.0-py3-none-any.whl (4.3 MB)
#12 4.287      ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 4.3/4.3 MB 22.4 MB/s eta 0:00:00
#12 4.346 Collecting platformdirs<5,>=3.9.1
#12 4.354   Downloading platformdirs-4.3.7-py3-none-any.whl (18 kB)
#12 4.398 Collecting filelock<4,>=3.12.2
#12 4.406   Downloading filelock-3.18.0-py3-none-any.whl (16 kB)
#12 4.427 Collecting distlib<1,>=0.3.7
#12 4.434   Downloading distlib-0.3.9-py2.py3-none-any.whl (468 kB)
#12 4.448      ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 469.0/469.0 KB 40.5 MB/s eta 0:00:00
#12 4.528 Installing collected packages: distlib, platformdirs, filelock, virtualenv
#12 4.710 Successfully installed distlib-0.3.9 filelock-3.18.0 platformdirs-4.3.7 virtualenv-20.30.0
#12 4.710 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
#12 DONE 4.8s

#13 [7/8] COPY host-x86_64/mingw-check/validate-toolstate.sh /scripts/
#13 DONE 0.0s
---
DirectMap4k:      126912 kB
DirectMap2M:     7213056 kB
DirectMap1G:    11534336 kB
##[endgroup]
Executing TIDY_PRINT_DIFF=1 python2.7 ../x.py test            --stage 0 src/tools/tidy tidyselftest --extra-checks=py,cpp
+ TIDY_PRINT_DIFF=1 python2.7 ../x.py test --stage 0 src/tools/tidy tidyselftest --extra-checks=py,cpp
##[group]Building bootstrap
    Finished `dev` profile [unoptimized] target(s) in 0.05s
##[endgroup]
WARN: currently no CI rustc builds have rustc debug assertions enabled. Please either set `rust.debug-assertions` to `false` if you want to use download CI rustc or set `rust.download-rustc` to `false`.
[TIMING] core::build_steps::tool::LibcxxVersionTool { target: x86_64-unknown-linux-gnu } -- 0.215
---
fmt check
fmt: checked 5982 files
tidy check
tidy: Skipping binary file check, read-only filesystem
##[error]tidy error: /checkout/compiler/rustc_parse/src/parser/expr.rs:3427: TODO is used for tasks that should be done before merging a PR; If you want to leave a message in the codebase use FIXME
removing old virtual environment
creating virtual environment at '/checkout/obj/build/venv' using 'python3.10' and 'venv'
creating virtual environment at '/checkout/obj/build/venv' using 'python3.10' and 'virtualenv'
Requirement already satisfied: pip in ./build/venv/lib/python3.10/site-packages (25.0.1)
linting python files
All checks passed!
checking python file formatting
26 files already formatted
checking C++ file formatting
some tidy checks failed
Command has failed. Rerun with -v to see more details.
Build completed unsuccessfully in 0:01:51
  local time: Wed Apr 23 12:04:34 UTC 2025
  network time: Wed, 23 Apr 2025 12:04:34 GMT
##[error]Process completed with exit code 1.
Post job cleanup.

= help: add `#![feature(let_chains)]` to the crate attributes to enable
= note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date

error[E0658]: `let` expressions in this position are unstable
Copy link
Member

Choose a reason for hiding this comment

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

Is this fine in the 2024 edition? If so it should say that rather than it being unstable.

Copy link
Member

Choose a reason for hiding this comment

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

It's due to the way #132833 was done: it just exempted 2024 usages from the feature gate instead of stabilizing the feature gate and adding an edition dependent error. I left that for future work, as I didn't know when it would stabilize. Plus, I saw that a lot of compiler crates were using let chains and I didn't want to play catch up with all the #![feature(let_chains)] changes happening that occur as the compiler does changes naturally (knowing that the stabilization would likely take months).

@est31
Copy link
Member

est31 commented Apr 23, 2025

if_let_guards don't have else clauses. The temporaries in the guards are dropped before the next match arm is encountered. So there is no rescoping edition difference there.

Same goes for while. I'd rather go in the opposite direction and allow let chains in while on all editions. Edit: #140204 .

@traviscross
Copy link
Contributor

@rustbot labels +I-lang-nominated

Let's vibe check on the lang call the direction we want to go here.

@rustbot rustbot added the I-lang-nominated Nominated for discussion during a lang team meeting. label Apr 23, 2025
@traviscross
Copy link
Contributor

@rustbot labels -I-lang-nominated

We discussed this today. Our general policy is to prefer consistency between editions over consistency within an edition, so our appetite was in favor of allowing what can be allowed in older editions. We should of course ensure the diagnostics in the older editions are helpful in mitigating the surprise this may cause, e.g. by having the errors note when something needs an edition bump.

The stabilizations for this will of course need our FCP.

@rustbot rustbot removed the I-lang-nominated Nominated for discussion during a lang team meeting. label Apr 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants