Pass -Werror when building the LLVM wrapper #143807

rperier · 2025-07-11T16:34:59Z

rustbot · 2025-07-11T16:35:04Z

rustbot has assigned @cuviper.
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

cuviper · 2025-07-11T16:40:43Z

I disagree with applying -Werror wholesale, because that can break anyone who happens to use a different C++ compiler than what we already tested with, especially for new warnings that may not even be real problems, or may be caused by external headers out of our control.

Maybe we could inject -Werror only for Rust CI, where we control the entire build environment.

rperier · 2025-07-11T16:54:10Z

Even if warnings are not enabled ? Well, even if you can still get warnings that are not parts of -Wall and -Wextra...
I understand now what @jyn514 meant by "in CI". I gonna to investigate to find how to inject this only for Rust CI.

Noratrieb · 2025-07-11T21:12:02Z

note that this here isn't actually building llvm, it's just building rustc's wrapper. the actual LLVM build happens in src/bootstrap

cuviper · 2025-07-11T21:21:13Z

I gonna to investigate to find how to inject this only for Rust CI.

I don't think it needs to be sophisticated -- maybe just check for the CI environment variable, which has precedent:

rust/compiler/rustc_codegen_llvm/src/llvm_util.rs

Line 55 in 855e0fe

if std::env::var_os("CI").is_some() {

Noratrieb · 2025-07-11T22:26:55Z

Bootstrap has existing methods for checking whether it's in rust-lang/rust CI, maybe you could reuse those by piping through environment variables or something like that

rperier · 2025-07-12T13:24:32Z

Boostrap has existing types and methods for checking whether it's in rust-lang/rust CI, that's the CiEnv enum. However, it seems that .github/workflows/ci.yml calls src/ci/docker/run.sh at some point. The docker script wrapper set the environment variable CI before running the container. That's the variable that is used later by compiler/rustc_codegen_llvm/src/llvm_util.rs. Checking for the presence of the CI environment variable looks simpler, as we already have everything we need, in my humble opinion.

cuviper · 2025-08-06T21:53:16Z

@bors r+ rollup=iffy

bors · 2025-08-06T21:53:19Z

📌 Commit 8271991 has been approved by cuviper

It is now in the queue for this repository.

Pass -Werror when building the LLVM wrapper cc rust-lang#109712

Rollup of 7 pull requests Successful merges: - #138689 (add nvptx_target_feature) - #140267 (implement continue_ok and break_ok for ControlFlow) - #143807 (Pass -Werror when building the LLVM wrapper) - #144369 (Upgrade semicolon_in_expressions_from_macros from warn to deny) - #144601 (Allow `cargo fix` to partially apply `mismatched_lifetime_syntaxes`) - #144682 (Stabilize `strict_overflow_ops`) - #145026 (Update books) r? `@ghost` `@rustbot` modify labels: rollup

Zalathar · 2025-08-07T06:32:07Z

Failed in rollup: #145031 (comment)

@bors r-

tgross35 · 2025-08-07T06:32:18Z

@bors r- #145031 (comment)

@bors2 try jobs=aarch64-msvc-1

rust-bors · 2025-08-07T06:32:23Z

⌛ Trying commit 8271991 with merge ebba86f…

To cancel the try build, run the command @bors try cancel.

Pass -Werror when building the LLVM wrapper try-job: aarch64-msvc-1

rust-bors · 2025-08-07T06:57:56Z

💔 Test failed (CI). Failed jobs:

bors build finished (web logs, extended logs)
try - aarch64-msvc-1 (web logs, extended logs)

rperier · 2025-08-07T08:20:12Z

Whooops, sorry, it seems that something went wrong. I guess that I am able to reproduce the issue by using the try command with bors (at least according to the doc). So either I exclude warnings_into_errors for these targets (by using the TARGET variable)... but it looks a workaround to me, or I fix the warnings (do you want to invest time for this ?)

rperier · 2025-08-07T12:39:56Z

@bors try jobs=aarch64-msvc-1

rust-bors · 2025-08-07T12:39:59Z

@rperier: 🔑 Insufficient privileges: not in try users

rperier · 2025-08-07T16:24:11Z

As this PR was intended to be merged as a trivial change, I propose the following fix, it is less intrusive than fixing the warnings (hope this helps as I cannot try the job via bors).

@rustbot ready

tgross35 · 2025-08-07T17:08:16Z

@bors2 try jobs=aarch64-msvc-1

Pass -Werror when building the LLVM wrapper try-job: aarch64-msvc-1

rust-bors · 2025-08-07T17:08:21Z

⌛ Trying commit 57491cb with merge e255101…

To cancel the try build, run the command @bors try cancel.

compiler/rustc_llvm/build.rs

Enabling warning_into_errors() only whether it's in rust-lang/rust CI, so deprecated uses of LLVM methods can be treated as errors.

rust-bors · 2025-08-07T19:17:19Z

☀️ Try build successful (CI)
Build commit: e255101 (e255101d6c5c5b93497c2d6860caa21624efcf3d, parent: cd434309efcf5dc68b253e5ef6ba40c1c43711c9)

cuviper · 2025-08-07T23:00:05Z

@bors r+

bors · 2025-08-07T23:00:08Z

📌 Commit d37fae3 has been approved by cuviper

It is now in the queue for this repository.

bors · 2025-08-07T23:30:02Z

⌛ Testing commit d37fae3 with merge a980cd4...

bors · 2025-08-08T02:58:25Z

☀️ Test successful - checks-actions
Approved by: cuviper
Pushing a980cd4 to master...

github-actions · 2025-08-08T03:01:54Z

What is this?

This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.

Comparing 2fd855f (parent) -> a980cd4 (this PR)

Test differences

Show 4 test diffs

4 doctest diffs were found. These are ignored, as they are noisy.

Test dashboard

Run

cargo run --manifest-path src/ci/citool/Cargo.toml -- \
    test-dashboard a980cd4311ae4b5bf9099d418e32643d068f1344 --output-dir test-dashboard

And then open test-dashboard/index.html in your browser to see an overview of all executed tests.

Job duration changes

x86_64-apple-1: 10634.4s -> 6526.5s (-38.6%)
x86_64-apple-2: 3565.5s -> 4757.7s (33.4%)
dist-x86_64-apple: 9494.9s -> 11888.7s (25.2%)
dist-powerpc64le-linux-musl: 5856.7s -> 5094.9s (-13.0%)
aarch64-apple: 6339.5s -> 6972.1s (10.0%)
dist-arm-linux-gnueabi: 5272.4s -> 4776.4s (-9.4%)
dist-loongarch64-musl: 5465.9s -> 4955.1s (-9.3%)
aarch64-gnu-llvm-19-1: 3478.3s -> 3167.5s (-8.9%)
dist-armv7-linux: 5523.5s -> 5062.7s (-8.3%)
aarch64-gnu: 7019.8s -> 6525.2s (-7.0%)

How to interpret the job duration changes?

Job durations can vary a lot, based on the actual runner instance
that executed the job, system noise, invalidated caches, etc. The table above is provided
mostly for t-infra members, for simpler debugging of potential CI slow-downs.

rust-timer · 2025-08-08T05:52:19Z

Finished benchmarking commit (a980cd4): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.6%	[-0.6%, -0.5%]	3
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results (secondary 3.2%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	3.2%	[3.2%, 3.2%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Cycles

Results (secondary 0.7%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	4.4%	[2.6%, 6.3%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-3.0%	[-3.7%, -2.2%]	2
All ❌✅ (primary)	-	-	0

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 465.394s -> 465.2s (-0.04%)
Artifact size: 377.41 MiB -> 377.41 MiB (0.00%)

rustbot assigned cuviper Jul 11, 2025

Noratrieb changed the title ~~Pass -Werror when building LLVM~~ Pass -Werror when building the LLVM wrapper Jul 12, 2025

rperier force-pushed the rustc_llvm_werror branch from ae1ee6e to 8271991 Compare July 12, 2025 14:49

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 6, 2025

tgross35 added a commit to tgross35/rust that referenced this pull request Aug 7, 2025

Rollup merge of rust-lang#143807 - rperier:rustc_llvm_werror, r=cuviper

6e13b10

Pass -Werror when building the LLVM wrapper cc rust-lang#109712

tgross35 mentioned this pull request Aug 7, 2025

Rollup of 7 pull requests #145031

Closed

bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 7, 2025

rust-bors bot added a commit that referenced this pull request Aug 7, 2025

Auto merge of #143807 - rperier:rustc_llvm_werror, r=<try>

ebba86f

Pass -Werror when building the LLVM wrapper try-job: aarch64-msvc-1

This comment has been minimized.

Sign in to view

rperier force-pushed the rustc_llvm_werror branch from 8271991 to 4ddb9ed Compare August 7, 2025 09:15

This comment has been minimized.

Sign in to view

rperier force-pushed the rustc_llvm_werror branch from 4ddb9ed to 57491cb Compare August 7, 2025 09:24

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 7, 2025

rust-bors bot added a commit that referenced this pull request Aug 7, 2025

Auto merge of #143807 - rperier:rustc_llvm_werror, r=<try>

e255101

Pass -Werror when building the LLVM wrapper try-job: aarch64-msvc-1

tgross35 reviewed Aug 7, 2025

View reviewed changes

compiler/rustc_llvm/build.rs Outdated Show resolved Hide resolved

Pass -Werror when building the LLVM wrapper

d37fae3

Enabling warning_into_errors() only whether it's in rust-lang/rust CI, so deprecated uses of LLVM methods can be treated as errors.

rperier force-pushed the rustc_llvm_werror branch from 57491cb to d37fae3 Compare August 7, 2025 18:34

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 7, 2025

bors added the merged-by-bors This PR was explicitly merged by bors. label Aug 8, 2025

bors merged commit a980cd4 into rust-lang:master Aug 8, 2025
11 checks passed

rustbot added this to the 1.91.0 milestone Aug 8, 2025

Pass -Werror when building the LLVM wrapper #143807

Pass -Werror when building the LLVM wrapper #143807

Uh oh!

Conversation

rperier commented Jul 11, 2025 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Jul 11, 2025

Uh oh!

cuviper commented Jul 11, 2025

Uh oh!

rperier commented Jul 11, 2025

Uh oh!

Noratrieb commented Jul 11, 2025

Uh oh!

cuviper commented Jul 11, 2025

Uh oh!

Noratrieb commented Jul 11, 2025

Uh oh!

rperier commented Jul 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cuviper commented Aug 6, 2025

Uh oh!

bors commented Aug 6, 2025

Uh oh!

Zalathar commented Aug 7, 2025

Uh oh!

tgross35 commented Aug 7, 2025

Uh oh!

rust-bors bot commented Aug 7, 2025

Uh oh!

This comment has been minimized.

rust-bors bot commented Aug 7, 2025

Uh oh!

rperier commented Aug 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment has been minimized.

rperier commented Aug 7, 2025

Uh oh!

rust-bors bot commented Aug 7, 2025

Uh oh!

rperier commented Aug 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tgross35 commented Aug 7, 2025

Uh oh!

rust-bors bot commented Aug 7, 2025

Uh oh!

Uh oh!

rust-bors bot commented Aug 7, 2025

Uh oh!

cuviper commented Aug 7, 2025

Uh oh!

bors commented Aug 7, 2025

Uh oh!

bors commented Aug 7, 2025

Uh oh!

bors commented Aug 8, 2025

Uh oh!

Uh oh!

github-actions bot commented Aug 8, 2025

Test differences

Job duration changes

Uh oh!

rust-timer commented Aug 8, 2025

Overall result: ✅ improvements - no action needed

Instruction count

Max RSS (memory usage)

Cycles

Binary size

Uh oh!

Uh oh!

rperier commented Jul 11, 2025 •

edited by rustbot

Loading

rperier commented Jul 12, 2025 •

edited

Loading

rperier commented Aug 7, 2025 •

edited

Loading

rperier commented Aug 7, 2025 •

edited

Loading