From d64aea65ad3b994ed0e1503f836d82a9896ab468 Mon Sep 17 00:00:00 2001 From: Nilstrieb <48135649+Nilstrieb@users.noreply.github.com> Date: Mon, 15 Nov 2021 20:40:29 +0100 Subject: [PATCH 1/4] Fix `non-constant value` ICE (#90878) This also fixes the same suggestion, which was kind of broken, because it just searched for the last occurence of `const` to replace with a `let`. This works great in some cases, but when there is no const and a leading space to the file, it doesn't work and panic with overflow because it thought that it had found a const. I also changed the suggestion to only trigger if the `const` and the non-constant value are on the same line, because if they aren't, the suggestion is very likely to be wrong. Also don't trigger the suggestion if the found `const` is on line 0, because that triggers the ICE. --- compiler/rustc_resolve/src/diagnostics.rs | 16 +++++++++- compiler/rustc_span/src/lib.rs | 1 + src/test/ui/consts/issue-90878-2.rs | 12 ++++++++ src/test/ui/consts/issue-90878-2.stderr | 30 +++++++++++++++++++ src/test/ui/consts/issue-90878.rs | 4 +++ src/test/ui/consts/issue-90878.stderr | 11 +++++++ .../ui/consts/non-const-value-in-const.rs | 7 +++++ .../ui/consts/non-const-value-in-const.stderr | 20 +++++++++++++ 8 files changed, 100 insertions(+), 1 deletion(-) create mode 100644 src/test/ui/consts/issue-90878-2.rs create mode 100644 src/test/ui/consts/issue-90878-2.stderr create mode 100644 src/test/ui/consts/issue-90878.rs create mode 100644 src/test/ui/consts/issue-90878.stderr create mode 100644 src/test/ui/consts/non-const-value-in-const.rs create mode 100644 src/test/ui/consts/non-const-value-in-const.stderr diff --git a/compiler/rustc_resolve/src/diagnostics.rs b/compiler/rustc_resolve/src/diagnostics.rs index c46a18e51031a..e9680aae3f89d 100644 --- a/compiler/rustc_resolve/src/diagnostics.rs +++ b/compiler/rustc_resolve/src/diagnostics.rs @@ -450,9 +450,23 @@ impl<'a> Resolver<'a> { // let foo =... // ^^^ given this Span // ------- get this Span to have an applicable suggestion + + // edit: + // only do this if the const and usage of the non-constant value are on the same line + // the further the two are apart, the higher the chance of the suggestion being wrong + // also make sure that this line isn't the first one (ICE #90878) + let sp = self.session.source_map().span_extend_to_prev_str(ident.span, current, true); - if sp.lo().0 == 0 { + + let is_first_line = self + .session + .source_map() + .lookup_line(sp.lo()) + .map(|file_and_line| file_and_line.line == 0) + .unwrap_or(true); + + if sp.lo().0 == 0 || self.session.source_map().is_multiline(sp) || is_first_line { err.span_label(ident.span, &format!("this would need to be a `{}`", sugg)); } else { let sp = sp.with_lo(BytePos(sp.lo().0 - current.len() as u32)); diff --git a/compiler/rustc_span/src/lib.rs b/compiler/rustc_span/src/lib.rs index dfc64f37e4c46..1445c59710cc3 100644 --- a/compiler/rustc_span/src/lib.rs +++ b/compiler/rustc_span/src/lib.rs @@ -1935,6 +1935,7 @@ pub struct Loc { #[derive(Debug)] pub struct SourceFileAndLine { pub sf: Lrc, + /// Index of line, starting from 0. pub line: usize, } #[derive(Debug)] diff --git a/src/test/ui/consts/issue-90878-2.rs b/src/test/ui/consts/issue-90878-2.rs new file mode 100644 index 0000000000000..7e3f18cc9d593 --- /dev/null +++ b/src/test/ui/consts/issue-90878-2.rs @@ -0,0 +1,12 @@ + #![l=|x|[b;x ]] //~ ERROR unexpected token: `|x| [b; x]` +//~^ ERROR cannot find attribute `l` in this scope +//~^^ ERROR attempt to use a non-constant value in a constant [E0435] +//~^^^ ERROR cannot find value `b` in this scope [E0425] + +// notice the space at the start, +// we can't attach any attributes to this file because it needs to be at the start + +// this example has been slightly modified (adding ]] at the end), so that it actually works here +// it still produces the same issue though + +fn main() {} diff --git a/src/test/ui/consts/issue-90878-2.stderr b/src/test/ui/consts/issue-90878-2.stderr new file mode 100644 index 0000000000000..9e167424995a7 --- /dev/null +++ b/src/test/ui/consts/issue-90878-2.stderr @@ -0,0 +1,30 @@ +error: unexpected token: `|x| [b; x]` + --> $DIR/issue-90878-2.rs:1:7 + | +LL | #![l=|x|[b;x ]] + | ^^^^^^^^^ + +error: cannot find attribute `l` in this scope + --> $DIR/issue-90878-2.rs:1:5 + | +LL | #![l=|x|[b;x ]] + | ^ + +error[E0435]: attempt to use a non-constant value in a constant + --> $DIR/issue-90878-2.rs:1:13 + | +LL | #![l=|x|[b;x ]] + | - ^ + | | + | this would need to be a `const` + +error[E0425]: cannot find value `b` in this scope + --> $DIR/issue-90878-2.rs:1:11 + | +LL | #![l=|x|[b;x ]] + | ^ help: a local variable with a similar name exists: `x` + +error: aborting due to 4 previous errors + +Some errors have detailed explanations: E0425, E0435. +For more information about an error, try `rustc --explain E0425`. diff --git a/src/test/ui/consts/issue-90878.rs b/src/test/ui/consts/issue-90878.rs new file mode 100644 index 0000000000000..43f6fe5f3800d --- /dev/null +++ b/src/test/ui/consts/issue-90878.rs @@ -0,0 +1,4 @@ + fn main() { + |x: usize| [0; x]; //~ ERROR attempt to use a non-constant value in a constant [E0435] + // (note the space before "fn") +} diff --git a/src/test/ui/consts/issue-90878.stderr b/src/test/ui/consts/issue-90878.stderr new file mode 100644 index 0000000000000..c038fc622d468 --- /dev/null +++ b/src/test/ui/consts/issue-90878.stderr @@ -0,0 +1,11 @@ +error[E0435]: attempt to use a non-constant value in a constant + --> $DIR/issue-90878.rs:2:20 + | +LL | |x: usize| [0; x]; + | - ^ + | | + | this would need to be a `const` + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0435`. diff --git a/src/test/ui/consts/non-const-value-in-const.rs b/src/test/ui/consts/non-const-value-in-const.rs new file mode 100644 index 0000000000000..1a20b1e09d7be --- /dev/null +++ b/src/test/ui/consts/non-const-value-in-const.rs @@ -0,0 +1,7 @@ +fn main() { + let x = 5; + const Y: i32 = x; //~ ERROR attempt to use a non-constant value in a constant [E0435] + + let x = 5; + let _ = [0; x]; //~ ERROR attempt to use a non-constant value in a constant [E0435] +} diff --git a/src/test/ui/consts/non-const-value-in-const.stderr b/src/test/ui/consts/non-const-value-in-const.stderr new file mode 100644 index 0000000000000..0ce4b4b705334 --- /dev/null +++ b/src/test/ui/consts/non-const-value-in-const.stderr @@ -0,0 +1,20 @@ +error[E0435]: attempt to use a non-constant value in a constant + --> $DIR/non-const-value-in-const.rs:3:20 + | +LL | const Y: i32 = x; + | ------- ^ non-constant value + | | + | help: consider using `let` instead of `const`: `let Y` + +error[E0435]: attempt to use a non-constant value in a constant + --> $DIR/non-const-value-in-const.rs:6:17 + | +LL | let x = 5; + | ----- help: consider using `const` instead of `let`: `const x` +... +LL | let _ = [0; x]; + | ^ non-constant value + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0435`. From 7c7f58d5b7b5b453fde743bb058aa5770a37e57f Mon Sep 17 00:00:00 2001 From: Nilstrieb <48135649+Nilstrieb@users.noreply.github.com> Date: Tue, 16 Nov 2021 22:16:47 +0100 Subject: [PATCH 2/4] Fix case where ICE #90878 was still triggered by a leading newline I cannot provide a test for that thanks to tidy. --- compiler/rustc_resolve/src/diagnostics.rs | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/compiler/rustc_resolve/src/diagnostics.rs b/compiler/rustc_resolve/src/diagnostics.rs index e9680aae3f89d..2e4cb4ff7270d 100644 --- a/compiler/rustc_resolve/src/diagnostics.rs +++ b/compiler/rustc_resolve/src/diagnostics.rs @@ -454,22 +454,20 @@ impl<'a> Resolver<'a> { // edit: // only do this if the const and usage of the non-constant value are on the same line // the further the two are apart, the higher the chance of the suggestion being wrong - // also make sure that this line isn't the first one (ICE #90878) + // also make sure that the pos for the suggestion is not 0 (ICE #90878) let sp = self.session.source_map().span_extend_to_prev_str(ident.span, current, true); - let is_first_line = self - .session - .source_map() - .lookup_line(sp.lo()) - .map(|file_and_line| file_and_line.line == 0) - .unwrap_or(true); + let pos_for_suggestion = sp.lo().0.saturating_sub(current.len() as u32); - if sp.lo().0 == 0 || self.session.source_map().is_multiline(sp) || is_first_line { + if sp.lo().0 == 0 + || pos_for_suggestion == 0 + || self.session.source_map().is_multiline(sp) + { err.span_label(ident.span, &format!("this would need to be a `{}`", sugg)); } else { - let sp = sp.with_lo(BytePos(sp.lo().0 - current.len() as u32)); + let sp = sp.with_lo(BytePos(pos_for_suggestion)); err.span_suggestion( sp, &format!("consider using `{}` instead of `{}`", sugg, current), From 495d8ed246ec659127ee646b55e0ea7874ecb82f Mon Sep 17 00:00:00 2001 From: Nilstrieb <48135649+Nilstrieb@users.noreply.github.com> Date: Wed, 17 Nov 2021 22:22:48 +0100 Subject: [PATCH 3/4] tidy-ignore-leading-newlines --- src/tools/tidy/src/style.rs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/tools/tidy/src/style.rs b/src/tools/tidy/src/style.rs index 022a3dfde8217..6ece94771401d 100644 --- a/src/tools/tidy/src/style.rs +++ b/src/tools/tidy/src/style.rs @@ -266,6 +266,8 @@ pub fn check(path: &Path, bad: &mut bool) { contains_ignore_directive(can_contain, &contents, "end-whitespace"); let mut skip_trailing_newlines = contains_ignore_directive(can_contain, &contents, "trailing-newlines"); + let mut skip_leading_newlines = + contains_ignore_directive(can_contain, &contents, "leading-newlines"); let mut skip_copyright = contains_ignore_directive(can_contain, &contents, "copyright"); let mut leading_new_lines = false; let mut trailing_new_lines = 0; @@ -350,7 +352,10 @@ pub fn check(path: &Path, bad: &mut bool) { } } if leading_new_lines { - tidy_error!(bad, "{}: leading newline", file.display()); + let mut err = |_| { + tidy_error!(bad, "{}: leading newline", file.display()); + }; + suppressible_tidy_err!(err, skip_leading_newlines, "mising leading newline"); } let mut err = |msg: &str| { tidy_error!(bad, "{}: {}", file.display(), msg); @@ -395,6 +400,9 @@ pub fn check(path: &Path, bad: &mut bool) { if let Directive::Ignore(false) = skip_trailing_newlines { tidy_error!(bad, "{}: ignoring trailing newlines unnecessarily", file.display()); } + if let Directive::Ignore(false) = skip_leading_newlines { + tidy_error!(bad, "{}: ignoring leading newlines unnecessarily", file.display()); + } if let Directive::Ignore(false) = skip_copyright { tidy_error!(bad, "{}: ignoring copyright unnecessarily", file.display()); } From 96c37c8a6a4bbfe34a81754e174bfc2e41d194f6 Mon Sep 17 00:00:00 2001 From: Nilstrieb <48135649+Nilstrieb@users.noreply.github.com> Date: Wed, 17 Nov 2021 22:24:42 +0100 Subject: [PATCH 4/4] Add a test with a leading newline for ICE #90878 --- src/test/ui/consts/issue-90878-3.rs | 6 ++++++ src/test/ui/consts/issue-90878-3.stderr | 11 +++++++++++ 2 files changed, 17 insertions(+) create mode 100644 src/test/ui/consts/issue-90878-3.rs create mode 100644 src/test/ui/consts/issue-90878-3.stderr diff --git a/src/test/ui/consts/issue-90878-3.rs b/src/test/ui/consts/issue-90878-3.rs new file mode 100644 index 0000000000000..0e36646eb49f5 --- /dev/null +++ b/src/test/ui/consts/issue-90878-3.rs @@ -0,0 +1,6 @@ + +fn main() { + |x: usize| [0; x]; //~ ERROR attempt to use a non-constant value in a constant [E0435] + // (note the newline before "fn") +} +// ignore-tidy-leading-newlines diff --git a/src/test/ui/consts/issue-90878-3.stderr b/src/test/ui/consts/issue-90878-3.stderr new file mode 100644 index 0000000000000..1bcc0eb37877b --- /dev/null +++ b/src/test/ui/consts/issue-90878-3.stderr @@ -0,0 +1,11 @@ +error[E0435]: attempt to use a non-constant value in a constant + --> $DIR/issue-90878-3.rs:3:20 + | +LL | |x: usize| [0; x]; + | - ^ + | | + | this would need to be a `const` + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0435`.