-
Notifications
You must be signed in to change notification settings - Fork 13.6k
Provide additional context to errors involving const traits #144194
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
base: master
Are you sure you want to change the base?
Changes from all commits
23cecd7
0fc3d5f
8b89cb3
90ff38b
fd07c3f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,9 +1,10 @@ | ||
//! Concrete error types for all operations which may be invalid in a certain const context. | ||
use hir::{ConstContext, LangItem}; | ||
use rustc_errors::Diag; | ||
use rustc_errors::codes::*; | ||
use rustc_errors::{Applicability, Diag, MultiSpan}; | ||
use rustc_hir as hir; | ||
use rustc_hir::def::DefKind; | ||
use rustc_hir::def_id::DefId; | ||
use rustc_infer::infer::TyCtxtInferExt; | ||
use rustc_infer::traits::{ImplSource, Obligation, ObligationCause}; | ||
|
@@ -211,6 +212,7 @@ fn build_error_for_const_call<'tcx>( | |
|
||
debug!(?call_kind); | ||
|
||
let mut note = true; | ||
let mut err = match call_kind { | ||
CallKind::Normal { desugaring: Some((kind, self_ty)), .. } => { | ||
macro_rules! error { | ||
|
@@ -355,20 +357,82 @@ fn build_error_for_const_call<'tcx>( | |
non_or_conditionally, | ||
}) | ||
} | ||
_ => ccx.dcx().create_err(errors::NonConstFnCall { | ||
span, | ||
def_descr: ccx.tcx.def_descr(callee), | ||
def_path_str: ccx.tcx.def_path_str_with_args(callee, args), | ||
kind: ccx.const_kind(), | ||
non_or_conditionally, | ||
}), | ||
_ => { | ||
let def_descr = ccx.tcx.def_descr(callee); | ||
let mut err = ccx.dcx().create_err(errors::NonConstFnCall { | ||
span, | ||
def_descr, | ||
def_path_str: ccx.tcx.def_path_str_with_args(callee, args), | ||
kind: ccx.const_kind(), | ||
non_or_conditionally, | ||
}); | ||
let context_span = ccx.tcx.def_span(ccx.def_id()); | ||
err.span_label(context_span, format!( | ||
"calls in {}s are limited to constant functions, tuple structs and tuple variants", | ||
ccx.const_kind(), | ||
)); | ||
note = false; | ||
let def_kind = ccx.tcx.def_kind(callee); | ||
if let DefKind::AssocTy | DefKind::AssocConst | DefKind::AssocFn = def_kind { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Callees are always |
||
let parent = ccx.tcx.parent(callee); | ||
if let DefKind::Trait = ccx.tcx.def_kind(parent) | ||
Comment on lines
+376
to
+378
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This whole check could be simplified if you looked at |
||
&& !ccx.tcx.is_const_trait(parent) | ||
{ | ||
let assoc_span = ccx.tcx.def_span(callee); | ||
let assoc_name = ccx.tcx.item_name(callee); | ||
let mut span: MultiSpan = ccx.tcx.def_span(parent).into(); | ||
span.push_span_label(assoc_span, format!("this {def_descr} is not const")); | ||
let trait_descr = ccx.tcx.def_descr(parent); | ||
let trait_span = ccx.tcx.def_span(parent); | ||
let trait_name = ccx.tcx.item_name(parent); | ||
span.push_span_label(trait_span, format!("this {trait_descr} is not const")); | ||
err.span_note( | ||
span, | ||
format!( | ||
"{def_descr} `{assoc_name}` is not const because {trait_descr} \ | ||
`{trait_name}` is not const", | ||
), | ||
); | ||
if parent.is_local() && ccx.tcx.sess.is_nightly_build() { | ||
if !ccx.tcx.features().const_trait_impl() { | ||
err.help( | ||
"add `#![feature(const_trait_impl)]` to the crate attributes to \ | ||
enable `#[const_trait]`", | ||
); | ||
} | ||
let indentation = ccx | ||
.tcx | ||
.sess | ||
.source_map() | ||
.indentation_before(trait_span) | ||
.unwrap_or_default(); | ||
err.span_suggestion_verbose( | ||
trait_span.shrink_to_lo(), | ||
format!("consider making trait `{trait_name}` const"), | ||
format!("#[const_trait]\n{indentation}"), | ||
Applicability::MachineApplicable, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is not |
||
); | ||
} else if !ccx.tcx.sess.is_nightly_build() { | ||
err.help("const traits are not yet supported on stable Rust"); | ||
} | ||
} | ||
} else if ccx.tcx.constness(callee) != hir::Constness::Const { | ||
let name = ccx.tcx.item_name(callee); | ||
err.span_note( | ||
ccx.tcx.def_span(callee), | ||
format!("{def_descr} `{name}` is not const"), | ||
); | ||
} | ||
err | ||
} | ||
}; | ||
|
||
err.note(format!( | ||
"calls in {}s are limited to constant functions, \ | ||
tuple structs and tuple variants", | ||
ccx.const_kind(), | ||
)); | ||
if note { | ||
err.note(format!( | ||
"calls in {}s are limited to constant functions, tuple structs and tuple variants", | ||
ccx.const_kind(), | ||
)); | ||
} | ||
|
||
err | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did you change this to a label only for this case, and keep it as a note for the rest? Having special logic changing it between a note and a label seems too special cased. I'd prefer if you reverted it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line is pretty long for a label too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wanted to point at the constant context in the output in some way, as it isn't always trivially obvious why a constant function was expected. As we already had a note talking about the const context I moved that same message over from the note to the label so we wouldn't need both.
It was
vs
We could instead have
I was just trying to keep the verbosity increase to a minimum.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think the users need this to be a label. There's a tradeoff here with verbosity and I think this both complicates the implementation by having that
note
boolean and also splits the presentation of the diagnostic.