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

Tweak some diagnostic spans #59084

Merged
merged 10 commits into from
Mar 24, 2019
13 changes: 13 additions & 0 deletions src/librustc/mir/interpret/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ impl<'a, 'gcx, 'tcx> ConstEvalErr<'tcx> {
tcx: TyCtxtAt<'a, 'gcx, 'tcx>,
message: &str,
lint_root: hir::HirId,
span: Option<Span>,
) -> ErrorHandled {
let lint = self.struct_generic(
tcx,
Expand All @@ -108,6 +109,18 @@ impl<'a, 'gcx, 'tcx> ConstEvalErr<'tcx> {
);
match lint {
Ok(mut lint) => {
if let Some(span) = span {
let primary_spans = lint.span.primary_spans().to_vec();
// point at the actual error as the primary span
lint.replace_span_with(span);
// point to the `const` statement as a secondary span
// they don't have any label
for sp in primary_spans {
if sp != span {
lint.span_label(sp, "");
}
}
}
lint.emit();
ErrorHandled::Reported
},
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_errors/diagnostic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -366,7 +366,7 @@ impl Diagnostic {
}],
}],
msg: msg.to_owned(),
style: SuggestionStyle::HideCodeInline,
style: SuggestionStyle::HideCodeAlways,
applicability,
});
self
Expand Down
5 changes: 3 additions & 2 deletions src/librustc_metadata/native_libs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,10 @@ impl<'a, 'tcx> ItemLikeVisitor<'tcx> for Collector<'a, 'tcx> {
"dylib" => cstore::NativeUnknown,
"framework" => cstore::NativeFramework,
k => {
struct_span_err!(self.tcx.sess, m.span, E0458,
struct_span_err!(self.tcx.sess, item.span(), E0458,
"unknown kind: `{}`", k)
.span_label(item.span(), "unknown kind").emit();
.span_label(item.span(), "unknown kind")
.span_label(m.span, "").emit();
cstore::NativeUnknown
}
};
Expand Down
2 changes: 2 additions & 0 deletions src/librustc_mir/const_eval.rs
Original file line number Diff line number Diff line change
Expand Up @@ -668,6 +668,7 @@ pub fn const_eval_raw_provider<'a, 'tcx>(
tcx.at(tcx.def_span(def_id)),
"any use of this value will cause an error",
hir_id,
Some(err.span),
)
},
// promoting runtime code is only allowed to error if it references broken constants
Expand All @@ -684,6 +685,7 @@ pub fn const_eval_raw_provider<'a, 'tcx>(
tcx.at(span),
"reaching this expression at runtime will panic or abort",
tcx.hir().as_local_hir_id(def_id).unwrap(),
Some(err.span),
)
}
// anything else (array lengths, enum initializers, constant patterns) are reported
Expand Down
1 change: 1 addition & 0 deletions src/librustc_mir/transform/const_prop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,7 @@ impl<'a, 'mir, 'tcx> ConstPropagator<'a, 'mir, 'tcx> {
self.ecx.tcx,
"this expression will panic at runtime",
lint_root,
None,
);
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_resolve/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4932,7 +4932,7 @@ impl<'a> Resolver<'a> {
Some((directive, _, true)) if should_remove_import && !directive.is_glob() => {
// Simple case - remove the entire import. Due to the above match arm, this can
// only be a single use so just remove it entirely.
err.span_suggestion(
err.tool_only_span_suggestion(
directive.use_span_with_attributes,
"remove unnecessary import",
String::new(),
Expand Down Expand Up @@ -5112,7 +5112,7 @@ impl<'a> Resolver<'a> {
// extra for the comma.
span.lo().0 - (prev_comma.as_bytes().len() as u32) - 1
));
err.span_suggestion(
err.tool_only_span_suggestion(
Copy link
Member

@davidtwco davidtwco Mar 11, 2019

Choose a reason for hiding this comment

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

I'm unsure about this change. I agree that it makes the error much cleaner, but without the help: ... message how would someone know that they can automatically have the problem fixed?

(as a slight aside, I think we should try use try cc groups like @rust-lang/wg-diagnostics when landing PRs like this or the one that introduced tool_only_span_suggestion - I had no idea this existed and it'd have been nice to follow along with it)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

tool_only_span_diagnostics was landed over the all hands because it was needed for a different change, so it's pretty recent.

Tools still will see the same json output, so vscode and rustfix would still be able to apply these. I guess I hadn't thought of the use case of someone running rustfix after running rustc iteratively. Should we mark these some other way? Cyan asterisk next to the error or something?

Copy link
Member

Choose a reason for hiding this comment

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

My only concern is that users who know when they see help: ... that it is something that can be automatically applied won't know that for this error.

Something like a cyan asterisk would be ideal to draw attention to that (it might even raise awareness of rustfix being able to fix these things if everyone's error messages had a cyan asterisk suddenly appear). I wouldn't block this PR on that, so I'll r+ this and we can file a follow-up.

Copy link
Member

Choose a reason for hiding this comment

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

Filed #59103.

span, message, String::new(), Applicability::MaybeIncorrect,
);
return;
Expand Down
22 changes: 14 additions & 8 deletions src/librustc_typeck/check/method/suggest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,13 +69,18 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
error: MethodError<'tcx>,
args: Option<&'gcx [hir::Expr]>,
) {
let orig_span = span;
let mut span = span;
// Avoid suggestions when we don't know what's going on.
if rcvr_ty.references_error() {
return;
}

let report_candidates = |err: &mut DiagnosticBuilder<'_>,
mut sources: Vec<CandidateSource>| {
let report_candidates = |
span: Span,
err: &mut DiagnosticBuilder<'_>,
mut sources: Vec<CandidateSource>,
| {
sources.sort();
sources.dedup();
// Dynamic limit to avoid hiding just one candidate, which is silly.
Expand Down Expand Up @@ -293,9 +298,10 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
err.emit();
return;
} else {
span = item_name.span;
let mut err = struct_span_err!(
tcx.sess,
item_name.span,
span,
E0599,
"no {} named `{}` found for type `{}` in the current scope",
item_kind,
Expand All @@ -305,7 +311,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
if let Some(suggestion) = suggestion {
// enum variant
err.span_suggestion(
item_name.span,
span,
"did you mean",
suggestion.to_string(),
Applicability::MaybeIncorrect,
Expand Down Expand Up @@ -392,7 +398,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
}
} else {
err.span_label(span, format!("{} not found in `{}`", item_kind, ty_str));
self.tcx.sess.trait_methods_not_found.borrow_mut().insert(span);
self.tcx.sess.trait_methods_not_found.borrow_mut().insert(orig_span);
}

if self.is_fn_ty(&rcvr_ty, span) {
Expand Down Expand Up @@ -434,9 +440,9 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
self.ty_to_string(actual), item_name));
}

report_candidates(&mut err, static_sources);
report_candidates(span, &mut err, static_sources);
} else if static_sources.len() > 1 {
report_candidates(&mut err, static_sources);
report_candidates(span, &mut err, static_sources);
}

if !unsatisfied_predicates.is_empty() {
Expand Down Expand Up @@ -481,7 +487,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
"multiple applicable items in scope");
err.span_label(span, format!("multiple `{}` found", item_name));

report_candidates(&mut err, sources);
report_candidates(span, &mut err, sources);
err.emit();
}

Expand Down
21 changes: 14 additions & 7 deletions src/libsyntax/parse/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5598,8 +5598,14 @@ impl<'a> Parser<'a> {

if !negative_bounds.is_empty() || was_negative {
let plural = negative_bounds.len() > 1;
let mut err = self.struct_span_err(negative_bounds,
"negative trait bounds are not supported");
let last_span = negative_bounds.last().map(|sp| *sp);
let mut err = self.struct_span_err(
negative_bounds,
"negative trait bounds are not supported",
);
if let Some(sp) = last_span {
err.span_label(sp, "negative trait bounds are not supported");
}
if let Some(bound_list) = colon_span {
let bound_list = bound_list.to(self.prev_span);
let mut new_bound_list = String::new();
Expand All @@ -5612,11 +5618,12 @@ impl<'a> Parser<'a> {
}
new_bound_list = new_bound_list.replacen(" +", ":", 1);
}
err.span_suggestion_short(bound_list,
&format!("remove the trait bound{}",
if plural { "s" } else { "" }),
new_bound_list,
Applicability::MachineApplicable);
err.span_suggestion_hidden(
bound_list,
&format!("remove the trait bound{}", if plural { "s" } else { "" }),
new_bound_list,
Applicability::MachineApplicable,
);
}
err.emit();
}
Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/array_const_index-0.stderr
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error: any use of this value will cause an error
--> $DIR/array_const_index-0.rs:2:1
--> $DIR/array_const_index-0.rs:2:16
|
LL | const B: i32 = (&A)[1];
| ^^^^^^^^^^^^^^^-------^
| ---------------^^^^^^^-
| |
| index out of bounds: the len is 0 but the index is 1
|
Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/array_const_index-1.stderr
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error: any use of this value will cause an error
--> $DIR/array_const_index-1.rs:2:1
--> $DIR/array_const_index-1.rs:2:16
|
LL | const B: i32 = A[1];
| ^^^^^^^^^^^^^^^----^
| ---------------^^^^-
| |
| index out of bounds: the len is 0 but the index is 1
|
Expand Down
4 changes: 1 addition & 3 deletions src/test/ui/associated-const/associated-const-no-item.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,7 @@ error[E0599]: no associated item named `ID` found for type `i32` in the current
--> $DIR/associated-const-no-item.rs:5:23
|
LL | const X: i32 = <i32>::ID;
| -------^^
| |
| associated item not found in `i32`
| ^^ associated item not found in `i32`
|
= help: items from traits can only be used if the trait is implemented and in scope
= note: the following trait defines an item `ID`, perhaps you need to implement it:
Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/bad/bad-extern-link-attrs.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,10 @@ LL | #[link(name = "")]
| ^^^^^^^^^^^^^^^^^^ empty name given

error[E0458]: unknown kind: `bar`
--> $DIR/bad-extern-link-attrs.rs:4:1
--> $DIR/bad-extern-link-attrs.rs:4:22
|
LL | #[link(name = "foo", kind = "bar")]
| ^^^^^^^^^^^^^^^^^^^^^------------^^
| ---------------------^^^^^^^^^^^^--
| |
| unknown kind

Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/bogus-tag.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ LL | enum Color { Rgb(isize, isize, isize), Rgba(isize, isize, isize, isize), }
| ---------- variant `Hsl` not found here
...
LL | Color::Hsl(h, s, l) => { println!("hsl"); }
| -------^^^--------- variant not found in `Color`
| ^^^ variant not found in `Color`

error: aborting due to previous error

Expand Down
20 changes: 10 additions & 10 deletions src/test/ui/consts/const-err-early.stderr
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error: any use of this value will cause an error
--> $DIR/const-err-early.rs:3:1
--> $DIR/const-err-early.rs:3:19
|
LL | pub const A: i8 = -std::i8::MIN;
| ^^^^^^^^^^^^^^^^^^-------------^
| ------------------^^^^^^^^^^^^^-
| |
| attempt to negate with overflow
|
Expand All @@ -13,34 +13,34 @@ LL | #![deny(const_err)]
| ^^^^^^^^^

error: any use of this value will cause an error
--> $DIR/const-err-early.rs:4:1
--> $DIR/const-err-early.rs:4:19
|
LL | pub const B: u8 = 200u8 + 200u8;
| ^^^^^^^^^^^^^^^^^^-------------^
| ------------------^^^^^^^^^^^^^-
| |
| attempt to add with overflow

error: any use of this value will cause an error
--> $DIR/const-err-early.rs:5:1
--> $DIR/const-err-early.rs:5:19
|
LL | pub const C: u8 = 200u8 * 4;
| ^^^^^^^^^^^^^^^^^^---------^
| ------------------^^^^^^^^^-
| |
| attempt to multiply with overflow

error: any use of this value will cause an error
--> $DIR/const-err-early.rs:6:1
--> $DIR/const-err-early.rs:6:19
|
LL | pub const D: u8 = 42u8 - (42u8 + 1);
| ^^^^^^^^^^^^^^^^^^-----------------^
| ------------------^^^^^^^^^^^^^^^^^-
| |
| attempt to subtract with overflow

error: any use of this value will cause an error
--> $DIR/const-err-early.rs:7:1
--> $DIR/const-err-early.rs:7:19
|
LL | pub const E: u8 = [5u8][1];
| ^^^^^^^^^^^^^^^^^^--------^
| ------------------^^^^^^^^-
| |
| index out of bounds: the len is 1 but the index is 1

Expand Down
16 changes: 8 additions & 8 deletions src/test/ui/consts/const-err-multi.stderr
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error: any use of this value will cause an error
--> $DIR/const-err-multi.rs:3:1
--> $DIR/const-err-multi.rs:3:19
|
LL | pub const A: i8 = -std::i8::MIN;
| ^^^^^^^^^^^^^^^^^^-------------^
| ------------------^^^^^^^^^^^^^-
| |
| attempt to negate with overflow
|
Expand All @@ -13,26 +13,26 @@ LL | #![deny(const_err)]
| ^^^^^^^^^

error: any use of this value will cause an error
--> $DIR/const-err-multi.rs:5:1
--> $DIR/const-err-multi.rs:5:19
|
LL | pub const B: i8 = A;
| ^^^^^^^^^^^^^^^^^^-^
| ------------------^-
| |
| referenced constant has errors

error: any use of this value will cause an error
--> $DIR/const-err-multi.rs:7:1
--> $DIR/const-err-multi.rs:7:19
|
LL | pub const C: u8 = A as u8;
| ^^^^^^^^^^^^^^^^^^-------^
| ------------------^^^^^^^-
| |
| referenced constant has errors

error: any use of this value will cause an error
--> $DIR/const-err-multi.rs:9:1
--> $DIR/const-err-multi.rs:9:19
|
LL | pub const D: i8 = 50 - A;
| ^^^^^^^^^^^^^^^^^^------^
| ------------------^^^^^^-
| |
| referenced constant has errors

Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/consts/const-err.stderr
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
warning: any use of this value will cause an error
--> $DIR/const-err.rs:10:1
--> $DIR/const-err.rs:10:17
|
LL | const FOO: u8 = [5u8][1];
| ^^^^^^^^^^^^^^^^--------^
| ----------------^^^^^^^^-
| |
| index out of bounds: the len is 1 but the index is 1
|
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
warning: any use of this value will cause an error
--> $DIR/conditional_array_execution.rs:5:1
--> $DIR/conditional_array_execution.rs:5:19
|
LL | const FOO: u32 = [X - Y, Y - X][(X < Y) as usize];
| ^^^^^^^^^^^^^^^^^^-----^^^^^^^^^^^^^^^^^^^^^^^^^^^
| ------------------^^^^^---------------------------
| |
| attempt to subtract with overflow
|
Expand Down
Loading