rust-lang
warn less about non-exhaustive in ffi
Bindgen allows generating #[non_exhaustive] #[repr(u32)] enums. This results in nonintuitive nonlocal improper_ctypes warnings, even when the types are otherwise perfectly valid in C.
Adjust for actual tooling expectations by avoiding warning on simple enums with only unit variants.
Closes https://github.com/rust-lang/rust/issues/116831
r? @petrochenkov
(rustbot has picked a reviewer for you, use r? to override)
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@v4' (SHA:b4ffde65f46336ab88eb53be808477a3936bae11)
Download action repository 'actions/upload-artifact@v3' (SHA:a8a3f3ad30e3422c9c7b888a15615d19a852ae32)
Complete job name: PR - mingw-check-tidy
git config --global core.autocrlf false
shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
---
GITHUB_ACTION=__run_7
GITHUB_ACTIONS=true
GITHUB_ACTION_REF=
GITHUB_ACTION_REPOSITORY=
GITHUB_ACTOR=workingjubilee
GITHUB_API_URL=https://api.github.com
GITHUB_BASE_REF=master
GITHUB_ENV=/home/runner/work/_temp/_runner_file_commands/set_env_b929fa06-ff05-4ca6-af8c-284e55c2f882
GITHUB_EVENT_NAME=pull_request
---
GITHUB_SERVER_URL=https://github.com
GITHUB_SHA=2d5add8eb0e58f27961f876a307ece5ca221d32a
GITHUB_STATE=/home/runner/work/_temp/_runner_file_commands/save_state_b929fa06-ff05-4ca6-af8c-284e55c2f882
GITHUB_STEP_SUMMARY=/home/runner/work/_temp/_runner_file_commands/step_summary_b929fa06-ff05-4ca6-af8c-284e55c2f882
GITHUB_TRIGGERING_ACTOR=workingjubilee
GITHUB_WORKFLOW_REF=rust-lang/rust/.github/workflows/ci.yml@refs/pull/116863/merge
GITHUB_WORKFLOW_SHA=2d5add8eb0e58f27961f876a307ece5ca221d32a
GITHUB_WORKSPACE=/home/runner/work/rust/rust
GOROOT_1_19_X64=/opt/hostedtoolcache/go/1.19.13/x64
---
Removing intermediate container a609d6a52781
---> 1606f2c5f4af
Step 6/10 : COPY host-x86_64/mingw-check/reuse-requirements.txt /tmp/
---> 28ba99f1a67a
Step 7/10 : RUN pip3 install --no-deps --no-cache-dir --require-hashes -r /tmp/reuse-requirements.txt && pip3 install virtualenv
Collecting binaryornot==0.4.4
Downloading binaryornot-0.4.4-py2.py3-none-any.whl (9.0 kB)
Collecting boolean-py==4.0
Downloading boolean.py-4.0-py3-none-any.whl (25 kB)
---
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=180117 sha256=2196c9034bf565528bbb1ee6dad4f753eb813f58822363e6b768f09c73e4d4ff
Stored in directory: /tmp/pip-ephem-wheel-cache-o_0s0u44/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
Not uninstalling setuptools at /usr/lib/python3/dist-packages, outside environment /usr
Can't uninstall 'setuptools'. No files were found to uninstall.
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
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
Collecting virtualenv
Downloading virtualenv-20.24.5-py3-none-any.whl (3.7 MB)
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 3.7/3.7 MB 64.8 MB/s eta 0:00:00
Collecting platformdirs<4,>=3.9.1
Downloading platformdirs-3.11.0-py3-none-any.whl (17 kB)
Collecting distlib<1,>=0.3.7
Downloading distlib-0.3.7-py2.py3-none-any.whl (468 kB)
Collecting filelock<4,>=3.12.2
Downloading filelock-3.12.4-py3-none-any.whl (11 kB)
Downloading filelock-3.12.4-py3-none-any.whl (11 kB)
Installing collected packages: distlib, platformdirs, filelock, virtualenv
Successfully installed distlib-0.3.7 filelock-3.12.4 platformdirs-3.11.0 virtualenv-20.24.5
Removing intermediate container 595539e3d66b
---> f205f2209388
Step 8/10 : COPY host-x86_64/mingw-check/validate-toolstate.sh /scripts/
---> 2c9130b6d20e
---> 2c9130b6d20e
Step 9/10 : COPY host-x86_64/mingw-check/validate-error-codes.sh /scripts/
---> bb7c28fc1907
Step 10/10 : ENV SCRIPT TIDY_PRINT_DIFF=1 python2.7 ../x.py test --stage 0 src/tools/tidy tidyselftest --extra-checks=py:lint
Removing intermediate container 299801460c75
---> 5bda618df18a
Successfully built 5bda618df18a
Successfully tagged rust-ci:latest
Successfully tagged rust-ci:latest
##[endgroup]
Built container sha256:5bda618df18a4aa38bf980cf6739a4b781ff2afc4e54011007e5c5044a906294
Uploading finished image sha256:5bda618df18a4aa38bf980cf6739a4b781ff2afc4e54011007e5c5044a906294 to https://ci-caches.rust-lang.org/docker/8849b25aebb63c7041ab10114da59fac9c6c89ff409673e53f6251b7e63c69daeaca7298d30885d05004ab27b231421908523f297222d07a53450f37e4691d72
IMAGE CREATED CREATED BY SIZE COMMENT
5bda618df18a 1 second ago /bin/sh -c #(nop) ENV SCRIPT=TIDY_PRINT_DIF… 0B
2c9130b6d20e 2 seconds ago /bin/sh -c #(nop) COPY file:078ea1d11e7b7cda… 367B
f205f2209388 3 seconds ago |1 DEBIAN_FRONTEND=noninteractive /bin/sh -c… 23.9MB
28ba99f1a67a 9 seconds ago /bin/sh -c #(nop) COPY file:ac591dd6bc5afa66… 5.33kB
1606f2c5f4af 10 seconds ago |1 DEBIAN_FRONTEND=noninteractive /bin/sh -c… 23.1MB
---
<missing> 12 days ago /bin/sh -c #(nop) LABEL org.opencontainers.… 0B
<missing> 12 days ago /bin/sh -c #(nop) ARG LAUNCHPAD_BUILD_ARCH 0B
<missing> 12 days ago /bin/sh -c #(nop) ARG RELEASE 0B
<botocore.awsrequest.AWSRequest object at 0x7f790418f450>
gzip: stdout: Broken pipe
xargs: docker: terminated by signal 13
https://ci-caches.rust-lang.org/docker/8849b25aebb63c7041ab10114da59fac9c6c89ff409673e53f6251b7e63c69daeaca7298d30885d05004ab27b231421908523f297222d07a53450f37e4691d72
sha256:5bda618df18a4aa38bf980cf6739a4b781ff2afc4e54011007e5c5044a906294
---
DirectMap4k: 169920 kB
DirectMap2M: 8218624 kB
DirectMap1G: 10485760 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 [unoptimized] target(s) in 0.03s
##[endgroup]
downloading https://ci-artifacts.rust-lang.org/rustc-builds-alt/09df6108c84fdec400043d99d9ee232336fd5a9f/rust-dev-nightly-x86_64-unknown-linux-gnu.tar.xz
extracting /checkout/obj/build/cache/llvm-09df6108c84fdec400043d99d9ee232336fd5a9f-true/rust-dev-nightly-x86_64-unknown-linux-gnu.tar.xz to /checkout/obj/build/x86_64-unknown-linux-gnu/ci-llvm
---
##[endgroup]
fmt check
tidy check
tidy: Skipping binary file check, read-only filesystem
##[error]tidy error: /checkout/tests/ui/rfcs/rfc-2008-non-exhaustive/improper_ctypes/extern_crate_improper.rs:9: line longer than 100 chars
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 (23.2.1)
Collecting pip
Obtaining dependency information for pip from https://files.pythonhosted.org/packages/e0/63/b428aaca15fcd98c39b07ca7149e24bc14205ad0f1c80ba2b01835aedde1/pip-23.3-py3-none-any.whl.metadata
Downloading pip-23.3-py3-none-any.whl.metadata (3.5 kB)
Downloading pip-23.3-py3-none-any.whl (2.1 MB)
Installing collected packages: pip
Attempting uninstall: pip
Found existing installation: pip 23.2.1
Uninstalling pip-23.2.1:
Uninstalling pip-23.2.1:
Successfully uninstalled pip-23.2.1
Successfully installed pip-23.3
Collecting black==23.3.0 (from -r /checkout/src/tools/tidy/config/requirements.txt (line 7))
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 1.7/1.7 MB 41.8 MB/s eta 0:00:00
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 1.7/1.7 MB 41.8 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)
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)
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)
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)
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 138.8 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
WARNING: There was an error checking the latest version of pip.
some tidy checks failed
Build completed unsuccessfully in 0:00:49
local time: Tue Oct 17 22:51:46 UTC 2023
network time: Tue, 17 Oct 2023 22:51:46 GMT
I suppose some tests might depend on not having their imports formatted, but this is silly...
When I started reading https://github.com/rust-lang/rust/issues/116831 my first reaction was "if NodeTag is so special, then it's fine to use allow", but the problem is that you cannot actually apply allow to the single location where it would be most appropriate - enum NodeTag definition.
From the issue it looks like what you want to express is "this enum is non_exhaustive, but I guarantee that no future variant additions will change its ABI, so it's fine to use it in C interfaces and improper_ctypes should not be reported".
Maybe non_exhaustive can be enhanced with some markers describing what we promise not to add, and it would be useful in other situations too, not just in improper_ctypes.
In the meantime the lint could be relaxed.
Warning on #[repr(C)] #[non_exhaustive] was an explicit lang team decision, so I'll send this back to lang team for reconsidering.
When I started reading https://github.com/rust-lang/rust/issues/116831 my first reaction was "if NodeTag is so special, then it's fine to use allow", but the problem is that you cannot actually apply allow to the single location where it would be most appropriate -
enum NodeTagdefinition.
Correct, if the... approximately entire linting system was reworked, as I understand it, I guess the allow-site could be in the upstream crate?
Well, not allow specifically, something else.
allow is supposed to be placed on the location where something suspicious happens, e.g. a FFI-unsafe type is used in FFI (i.e. fn rdb_read_page in the linked issue).
What we want here is to mark NodeTag as FFI-safe, so it's never suspicious when used in any location.
Right, an arbitrary #[no_really_let_me_promise_a_semistable_abi_yet_nonexhaustive_for_reasons_that_are_really_long_to_explain] technically also works. That solution does not "spark joy", I think, but I can accept it.
:umbrella: The latest upstream changes (presumably #124558) made this pull request unmergeable. Please resolve the merge conflicts.
@rustbot labels -I-lang-nominated
We discussed this issue in the meeting on 2024-06-12. Our consensus was as follows:
- The intent of "non-exhaustive" (on enums) is to say "ensure that code will continue to compile if new variants are added".
- For lints, we generally try to ensure that you also won't get new lints.
- But lints are lints, we can say it's ok if "there exists extensions that would be legal" and "the ones that would get you a new lint are unlikely".
- In the case:
- We judge that adding a new data-carrying variant to an existing C-like enum that is passed to C is "unlikely", so we don't need the lint to account for it.
- E.g. going from
enum Foo { A, B, C }toenum Foo { A, B, C, D(u32) }.
- E.g. going from
- We judge that adding a new data-carrying variant to an existing C-like enum that is passed to C is "unlikely", so we don't need the lint to account for it.
- Additional consensus items:
- Non-exhaustive currently means code must be prepared for:
- On enums: new variants with any set of fields, public or private, etc.
- On structs or enum variants themselves: can add new fields with any type, public or private, etc.
- You cannot have values of an enum outside of the declared variants regardless of
#[non_exhaustive], that would require a fresh feature.
- Non-exhaustive currently means code must be prepared for:
- Possible future work:
- Extensions or revamped non-exhaustive that lets you be more specific about the range of future changes.
- A way to say that an enum may have variants not listed (e.g., due to C code).
Thank you, I will rebase this shortly and write about-as-much down into rustc_lint.
Done. Did a small amount of factoring out the logic into a module because I frankly can't imagine finding the decisions made within the thousands of lines of various lint code right now.
:pushpin: Commit 64f5736ec9c84161417044a892c49a6cf8e823dc has been approved by jieyouxu
It is now in the queue for this repository.