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

Change unexpected_cfg to allow by default #124841

Closed
wants to merge 1 commit into from

Conversation

tbu-
Copy link
Contributor

@tbu- tbu- commented May 7, 2024

This allows to find solutions to the false positives that were found in the ecosystem before turning it to warn by default again.

Most projects hit by this seem to just disable the warning, which indicates that it isn't working as expected.

CC #124800
CC #124804
CC #124821
CC hyperium/hyper#3660
CC microsoft/windows-rs#3022
CC rust-bitcoin/rust-bitcoin#2748
CC tokio-rs/tokio#6538

This allows to find solutions to the false positives that were found in
the ecosystem before turning it to `warn` by default again.

Most projects hit by this seem to just disable the warning, which
indicates that it isn't working as expected.

CC rust-lang#124800
CC rust-lang#124804
CC rust-lang#124821
CC hyperium/hyper#3660
CC microsoft/windows-rs#3022
CC rust-bitcoin/rust-bitcoin#2748
CC tokio-rs/tokio#6538
@rustbot
Copy link
Collaborator

rustbot commented May 7, 2024

r? @lcnr

rustbot has assigned @lcnr.
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 May 7, 2024
@rustbot
Copy link
Collaborator

rustbot commented May 7, 2024

Some changes occurred in tests/ui/check-cfg

cc @Urgau

@rust-log-analyzer
Copy link
Collaborator

The job x86_64-gnu-llvm-17 failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
#17 exporting to docker image format
#17 sending tarball 33.8s done
#17 DONE 38.7s
##[endgroup]
Setting extra environment values for docker:  --env ENABLE_GCC_CODEGEN=1 --env GCC_EXEC_PREFIX=/usr/lib/gcc/
[CI_JOB_NAME=x86_64-gnu-llvm-17]
---
sccache: Starting the server...
##[group]Configure the build
configure: processing command line
configure: 
configure: build.configure-args := ['--build=x86_64-unknown-linux-gnu', '--llvm-root=/usr/lib/llvm-17', '--enable-llvm-link-shared', '--set', 'rust.thin-lto-import-instr-limit=10', '--set', 'change-id=99999999', '--enable-verbose-configure', '--enable-sccache', '--disable-manage-submodules', '--enable-locked-deps', '--enable-cargo-native-static', '--set', 'rust.codegen-units-std=1', '--set', 'dist.compression-profile=balanced', '--dist-compression-formats=xz', '--disable-dist-src', '--release-channel=nightly', '--enable-debug-assertions', '--enable-overflow-checks', '--enable-llvm-assertions', '--set', 'rust.verify-llvm-ir', '--set', 'rust.codegen-backends=llvm,cranelift,gcc', '--set', 'llvm.static-libstdcpp', '--enable-new-symbol-mangling']
configure: target.x86_64-unknown-linux-gnu.llvm-config := /usr/lib/llvm-17/bin/llvm-config
configure: llvm.link-shared     := True
configure: rust.thin-lto-import-instr-limit := 10
configure: change-id            := 99999999
---
  Downloaded boml v0.3.1
   Compiling boml v0.3.1
   Compiling y v0.1.0 (/checkout/compiler/rustc_codegen_gcc/build_system)
    Finished `release` profile [optimized] target(s) in 3.50s
     Running `/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-codegen/x86_64-unknown-linux-gnu/release/y test --use-system-gcc --use-backend gcc --out-dir /checkout/obj/build/x86_64-unknown-linux-gnu/stage1-tools/cg_gcc --release --mini-tests --std-tests`
Using system GCC
[BUILD] example
[AOT] mini_core_hello_world
/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-tools/cg_gcc/mini_core_hello_world
abc
---
##[group]Building stage0 tool linkchecker (x86_64-unknown-linux-gnu)
    Finished `release` profile [optimized] target(s) in 0.13s
##[endgroup]
##[group]Testing stage0 Linkcheck (x86_64-unknown-linux-gnu)
cargo/print.html:5396: broken link fragment `#unexpected-cfgs` pointing to `rustc/lints/listing/warn-by-default.html`
cargo/print.html:5411: broken link fragment `#unexpected-cfgs` pointing to `rustc/lints/listing/warn-by-default.html`
cargo/reference/build-scripts.html:370: broken link fragment `#unexpected-cfgs` pointing to `rustc/lints/listing/warn-by-default.html`
cargo/reference/build-scripts.html:385: broken link fragment `#unexpected-cfgs` pointing to `rustc/lints/listing/warn-by-default.html`
number of HTML files scanned: 38355
number of HTML redirects found: 12087
number of links checked: 3070942
number of links ignored due to external: 73485

@Urgau
Copy link
Member

Urgau commented May 7, 2024

Most projects hit by this seem to just disable the warning, which indicates that it isn't working as expected.

This just represent some projects which are far from being representative of the ecosystem.

We also have received appreciation that the feature is working as expected.

Anyway, this PR premature to say the least, discussion is still happening on #124800.

@rustbot label +S-blocked -S-waiting-on-review

@rustbot rustbot 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 May 7, 2024
@tbu-
Copy link
Contributor Author

tbu- commented May 7, 2024

We also have received appreciation that the feature is working as expected.

Note that this isn't removing the feature altogether. I believe that one could significantly improve the false-positive rate and/or make creating exceptions easier so that the feature could be appreciated by everyone. I'm sure the projects mentioned above (like e.g. tokio) also like getting warnings about actually wrong #[cfg(…)].

@epage
Copy link
Contributor

epage commented May 7, 2024

imo this is a conversation best to be had on the issue to not fragment thingsd

@bors
Copy link
Contributor

bors commented May 9, 2024

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

@tbu-
Copy link
Contributor Author

tbu- commented May 20, 2024

Superseded by #125219.

@tbu- tbu- closed this May 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-blocked Status: Blocked on something else such as an RFC or other implementation work. 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