Skip to content

Conversation

ZuseZ4
Copy link
Member

@ZuseZ4 ZuseZ4 commented Jun 21, 2025

I still intend to finish #137570 by adding proper handling for library builds.
I was hoping to directly get the proper fix in, but I haven't had the time in a while, and other contributors and early testers run into this issue often enough.

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jun 21, 2025
@ZuseZ4 ZuseZ4 marked this pull request as ready for review June 25, 2025 17:06
@rustbot
Copy link
Collaborator

rustbot commented Jun 25, 2025

r? @oli-obk

rustbot has assigned @oli-obk.
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 the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 25, 2025
@rustbot
Copy link
Collaborator

rustbot commented Jun 25, 2025

Some changes occurred in compiler/rustc_codegen_ssa

cc @WaffleLapkin

@oli-obk
Copy link
Contributor

oli-obk commented Jun 25, 2025

Please give the commit a proper commit message 😆

@oli-obk oli-obk changed the title nicer error handling nicer autodiff error handling Jun 25, 2025
@ZuseZ4 ZuseZ4 force-pushed the autodiff-error-handling branch from acde7fc to f21fac3 Compare June 26, 2025 22:59
@ZuseZ4
Copy link
Member Author

ZuseZ4 commented Jun 26, 2025

cc @Sa4dUs that should prevent your issue in the future.

@ZuseZ4 ZuseZ4 force-pushed the autodiff-error-handling branch from f21fac3 to 5c4b6cc Compare June 27, 2025 00:06
@ZuseZ4
Copy link
Member Author

ZuseZ4 commented Jun 27, 2025

ok, I'm happy with these two tests, let me know if you want any changes.

@ZuseZ4 ZuseZ4 force-pushed the autodiff-error-handling branch from 5c4b6cc to 3610d7c Compare June 27, 2025 00:12
@@ -419,7 +419,12 @@ fn generate_lto_work<B: ExtraBackendMethods>(
} else {
if !autodiff.is_empty() {
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe pick a random (like the first) entry from this list and use its span so in case of a larger crate the user is aware of why this is being emitted. I could see it being confusing in case someone is adding cfgs to turn on/off autodiff and got sth wrong in the process

Copy link
Member Author

Choose a reason for hiding this comment

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

I added a span to the error, but emit_error now fails with:

thread 'coordinator' panicked at compiler/rustc_codegen_ssa/src/back/write.rs:1908:9:
assertion `left == right` failed
  left: MultiSpan { primary_spans: [Span { lo: BytePos(123), hi: BytePos(148), ctxt: #0 }], span_labels: [] }
 right: MultiSpan { primary_spans: [], span_labels: [] }
stack backtrace:

Do you have a recommendation for how to best emit the error? I had some issues to find the right function to get it to work.

Copy link
Member Author

Choose a reason for hiding this comment

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

ping @oli-obk if you by chance know the source of this error? Otherwise I can spend some more time to try it out.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know this error, maybe I'll see sth if you push the changes that don't work

@rustbot rustbot added the F-autodiff `#![feature(autodiff)]` label Jul 1, 2025
@rust-log-analyzer
Copy link
Collaborator

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

Click to see the possible cause of the failure (guessed by this bot)
 - LegacyKeyValueFormat: "ENV key=value" should be used instead of legacy "ENV key value" format (line 48)
 - LegacyKeyValueFormat: "ENV key=value" should be used instead of legacy "ENV key value" format (line 51)
 - LegacyKeyValueFormat: "ENV key=value" should be used instead of legacy "ENV key value" format (line 68)
##[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-19]
[CI_JOB_NAME=x86_64-gnu-llvm-19]
debug: `DISABLE_CI_RUSTC_IF_INCOMPATIBLE` configured.
---
sccache: Listening on address 127.0.0.1:4226
##[group]Configure the build
configure: processing command line
configure: 
configure: build.configure-args := ['--build=x86_64-unknown-linux-gnu', '--llvm-root=/usr/lib/llvm-19', '--enable-llvm-link-shared', '--set', 'rust.randomize-layout=true', '--set', 'rust.thin-lto-import-instr-limit=10', '--set', 'build.print-step-timings', '--enable-verbose-tests', '--set', 'build.metrics', '--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', '--set', 'rust.lld=false', '--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', '--set', 'gcc.download-ci-gcc=true', '--enable-new-symbol-mangling']
configure: build.build          := x86_64-unknown-linux-gnu
configure: target.x86_64-unknown-linux-gnu.llvm-config := /usr/lib/llvm-19/bin/llvm-config
configure: llvm.link-shared     := True
configure: rust.randomize-layout := True
configure: rust.thin-lto-import-instr-limit := 10
---
[RUSTC-TIMING] rustc_hir_analysis test:false 55.549
   Compiling rustc_hir_typeck v0.0.0 (/checkout/compiler/rustc_hir_typeck)
[RUSTC-TIMING] rustc_mir_build test:false 30.404
   Compiling rustc_mir_transform v0.0.0 (/checkout/compiler/rustc_mir_transform)
error[E0277]: the trait bound `MultiSpan: From<AutodiffLibraryBuild>` is not satisfied
    --> compiler/rustc_codegen_ssa/src/back/write.rs:424:41
     |
424  |                 dcx.handle().span_fatal(AutodiffLibraryBuild { span });
     |                              ---------- ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `From<AutodiffLibraryBuild>` is not implemented for `MultiSpan`
     |                              |
     |                              required by a bound introduced by this call
     |
     = help: the following other types implement trait `From<T>`:
               `MultiSpan` implements `From<Vec<rustc_span::Span>>`
               `MultiSpan` implements `From<rustc_span::Span>`
     = note: required for `AutodiffLibraryBuild` to implement `Into<MultiSpan>`
note: required by a bound in `DiagCtxtHandle::<'a>::span_fatal`
    --> /checkout/compiler/rustc_errors/src/lib.rs:1272:40
     |
1272 |     pub fn span_fatal(self, span: impl Into<MultiSpan>, msg: impl Into<DiagMessage>) -> ! {
     |                                        ^^^^^^^^^^^^^^^ required by this bound in `DiagCtxtHandle::<'a>::span_fatal`

error[E0061]: this method takes 2 arguments but 1 argument was supplied
    --> compiler/rustc_codegen_ssa/src/back/write.rs:424:30
     |
424  |                 dcx.handle().span_fatal(AutodiffLibraryBuild { span });
     |                              ^^^^^^^^^^------------------------------- argument #2 is missing
     |
note: method defined here
    --> /checkout/compiler/rustc_errors/src/lib.rs:1272:12
     |
1272 |     pub fn span_fatal(self, span: impl Into<MultiSpan>, msg: impl Into<DiagMessage>) -> ! {
     |            ^^^^^^^^^^
help: provide the argument
     |
424  |                 dcx.handle().span_fatal(AutodiffLibraryBuild { span }, /* msg */);
     |                                                                      +++++++++++

Some errors have detailed explanations: E0061, E0277.
For more information about an error, try `rustc --explain E0061`.
[RUSTC-TIMING] rustc_codegen_ssa test:false 3.390

@bors
Copy link
Collaborator

bors commented Jul 17, 2025

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

if cgcx.crate_types.contains(&CrateType::Rlib) {
dcx.handle().emit_fatal(AutodiffLibraryBuild {});
dcx.handle().span_fatal(AutodiffLibraryBuild { span });
Copy link
Contributor

Choose a reason for hiding this comment

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

This needs to stay `emit_fatal, unsure what span_fatal does

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
F-autodiff `#![feature(autodiff)]` 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.

5 participants