Skip to content

nicer autodiff error handling #142842

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 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions compiler/rustc_ast/src/expand/autodiff_attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
use std::fmt::{self, Display, Formatter};
use std::str::FromStr;

use rustc_span::Span;

use crate::expand::{Decodable, Encodable, HashStable_Generic};
use crate::ptr::P;
use crate::{Ty, TyKind};
Expand Down Expand Up @@ -85,6 +87,7 @@ pub struct AutoDiffItem {
/// The name of the function being generated
pub target: String,
pub attrs: AutoDiffAttrs,
pub span: Span,
}

#[derive(Clone, Eq, PartialEq, Encodable, Decodable, Debug, HashStable_Generic)]
Expand Down Expand Up @@ -276,8 +279,8 @@ impl AutoDiffAttrs {
!matches!(self.mode, DiffMode::Error | DiffMode::Source)
}

pub fn into_item(self, source: String, target: String) -> AutoDiffItem {
AutoDiffItem { source, target, attrs: self }
pub fn into_item(self, source: String, target: String, span: Span) -> AutoDiffItem {
AutoDiffItem { source, target, attrs: self, span }
}
}

Expand Down
5 changes: 3 additions & 2 deletions compiler/rustc_codegen_ssa/src/back/write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -419,11 +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

let dcx = cgcx.create_dcx();
let span = autodiff[0].span;
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

}
if cgcx.lto != Lto::Fat {
dcx.handle().emit_fatal(AutodiffWithoutLto {});
dcx.handle().emit_fatal(AutodiffWithoutLto { span });
}
}
assert!(needs_fat_lto.is_empty());
Expand Down
10 changes: 8 additions & 2 deletions compiler/rustc_codegen_ssa/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,17 @@ pub(crate) struct CguNotRecorded<'a> {

#[derive(Diagnostic)]
#[diag(codegen_ssa_autodiff_without_lto)]
pub struct AutodiffWithoutLto;
pub struct AutodiffWithoutLto {
#[primary_span]
pub span: Span,
}

#[derive(Diagnostic)]
#[diag(codegen_ssa_autodiff_lib_unsupported)]
pub struct AutodiffLibraryBuild;
pub struct AutodiffLibraryBuild {
#[primary_span]
pub span: Span,
}

#[derive(Diagnostic)]
#[diag(codegen_ssa_unknown_reuse_kind)]
Expand Down
5 changes: 3 additions & 2 deletions compiler/rustc_monomorphize/src/partitioning/autodiff.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,15 +120,16 @@ pub(crate) fn find_autodiff_source_functions<'tcx>(
None => continue,
};

debug!("source_id: {:?}", inst.def_id());
let source_def_id = inst.def_id();
debug!("source_id: {:?}", source_def_id);
let fn_ty = inst.ty(tcx, ty::TypingEnv::fully_monomorphized());
assert!(fn_ty.is_fn());
adjust_activity_to_abi(tcx, fn_ty, &mut input_activities);
let symb = symbol_name_for_instance_in_crate(tcx, inst.clone(), LOCAL_CRATE);

let mut new_target_attrs = target_attrs.clone();
new_target_attrs.input_activity = input_activities;
let itm = new_target_attrs.into_item(symb, target_symbol);
let itm = new_target_attrs.into_item(symb, target_symbol, tcx.def_span(source_def_id));
autodiff_items.push(itm);
}

Expand Down
Loading