Skip to content

Commit

Permalink
Rollup merge of rust-lang#91607 - FabianWolff:issue-91560-const-span,…
Browse files Browse the repository at this point in the history
… r=jackh726

Make `span_extend_to_prev_str()` more robust

Fixes rust-lang#91560. The logic in `span_extend_to_prev_str()` is currently quite brittle and fails if there is extra whitespace or something else in between, and it also should return an `Option` but doesn't currently.
  • Loading branch information
matthiaskrgr authored Feb 11, 2022
2 parents 5ccee0e + 71ff16b commit 9bbd8ec
Show file tree
Hide file tree
Showing 7 changed files with 123 additions and 46 deletions.
38 changes: 19 additions & 19 deletions compiler/rustc_resolve/src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -453,28 +453,28 @@ 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 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 pos_for_suggestion = sp.lo().0.saturating_sub(current.len() as u32);
let sp = self
.session
.source_map()
.span_extend_to_prev_str(ident.span, current, true, false);

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(pos_for_suggestion));
err.span_suggestion(
sp,
&format!("consider using `{}` instead of `{}`", sugg, current),
format!("{} {}", sugg, ident),
Applicability::MaybeIncorrect,
);
err.span_label(span, "non-constant value");
match sp {
Some(sp) if !self.session.source_map().is_multiline(sp) => {
let sp = sp.with_lo(BytePos(sp.lo().0 - (current.len() as u32)));
err.span_suggestion(
sp,
&format!("consider using `{}` instead of `{}`", sugg, current),
format!("{} {}", sugg, ident),
Applicability::MaybeIncorrect,
);
err.span_label(span, "non-constant value");
}
_ => {
err.span_label(ident.span, &format!("this would need to be a `{}`", sugg));
}
}

err
}
ResolutionError::BindingShadowsSomethingUnacceptable {
Expand Down
40 changes: 27 additions & 13 deletions compiler/rustc_span/src/source_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -629,26 +629,41 @@ impl SourceMap {
}

/// Extends the given `Span` to just after the previous occurrence of `pat` when surrounded by
/// whitespace. Returns the same span if no character could be found or if an error occurred
/// while retrieving the code snippet.
pub fn span_extend_to_prev_str(&self, sp: Span, pat: &str, accept_newlines: bool) -> Span {
/// whitespace. Returns None if the pattern could not be found or if an error occurred while
/// retrieving the code snippet.
pub fn span_extend_to_prev_str(
&self,
sp: Span,
pat: &str,
accept_newlines: bool,
include_whitespace: bool,
) -> Option<Span> {
// assure that the pattern is delimited, to avoid the following
// fn my_fn()
// ^^^^ returned span without the check
// ---------- correct span
let prev_source = self.span_to_prev_source(sp).ok()?;
for ws in &[" ", "\t", "\n"] {
let pat = pat.to_owned() + ws;
if let Ok(prev_source) = self.span_to_prev_source(sp) {
let prev_source = prev_source.rsplit(&pat).next().unwrap_or("").trim_start();
if prev_source.is_empty() && sp.lo().0 != 0 {
return sp.with_lo(BytePos(sp.lo().0 - 1));
} else if accept_newlines || !prev_source.contains('\n') {
return sp.with_lo(BytePos(sp.lo().0 - prev_source.len() as u32));
if let Some(pat_pos) = prev_source.rfind(&pat) {
let just_after_pat_pos = pat_pos + pat.len() - 1;
let just_after_pat_plus_ws = if include_whitespace {
just_after_pat_pos
+ prev_source[just_after_pat_pos..]
.find(|c: char| !c.is_whitespace())
.unwrap_or(0)
} else {
just_after_pat_pos
};
let len = prev_source.len() - just_after_pat_plus_ws;
let prev_source = &prev_source[just_after_pat_plus_ws..];
if accept_newlines || !prev_source.trim_start().contains('\n') {
return Some(sp.with_lo(BytePos(sp.lo().0 - len as u32)));
}
}
}

sp
None
}

/// Returns the source snippet as `String` after the given `Span`.
Expand Down Expand Up @@ -927,7 +942,7 @@ impl SourceMap {
}

pub fn generate_fn_name_span(&self, span: Span) -> Option<Span> {
let prev_span = self.span_extend_to_prev_str(span, "fn", true);
let prev_span = self.span_extend_to_prev_str(span, "fn", true, true).unwrap_or(span);
if let Ok(snippet) = self.span_to_snippet(prev_span) {
debug!(
"generate_fn_name_span: span={:?}, prev_span={:?}, snippet={:?}",
Expand Down Expand Up @@ -968,8 +983,7 @@ impl SourceMap {
pub fn generate_local_type_param_snippet(&self, span: Span) -> Option<(Span, String)> {
// Try to extend the span to the previous "fn" keyword to retrieve the function
// signature.
let sugg_span = self.span_extend_to_prev_str(span, "fn", false);
if sugg_span != span {
if let Some(sugg_span) = self.span_extend_to_prev_str(span, "fn", false, true) {
if let Ok(snippet) = self.span_to_snippet(sugg_span) {
// Consume the function name.
let mut offset = snippet
Expand Down
14 changes: 7 additions & 7 deletions src/test/ui/asm/aarch64/parse-error.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -382,7 +382,7 @@ error[E0435]: attempt to use a non-constant value in a constant
--> $DIR/parse-error.rs:39:37
|
LL | let mut foo = 0;
| ---------- help: consider using `const` instead of `let`: `const foo`
| ----------- help: consider using `const` instead of `let`: `const foo`
...
LL | asm!("{}", options(), const foo);
| ^^^ non-constant value
Expand All @@ -391,7 +391,7 @@ error[E0435]: attempt to use a non-constant value in a constant
--> $DIR/parse-error.rs:48:44
|
LL | let mut foo = 0;
| ---------- help: consider using `const` instead of `let`: `const foo`
| ----------- help: consider using `const` instead of `let`: `const foo`
...
LL | asm!("{}", clobber_abi("C"), const foo);
| ^^^ non-constant value
Expand All @@ -400,7 +400,7 @@ error[E0435]: attempt to use a non-constant value in a constant
--> $DIR/parse-error.rs:55:31
|
LL | let mut foo = 0;
| ---------- help: consider using `const` instead of `let`: `const foo`
| ----------- help: consider using `const` instead of `let`: `const foo`
...
LL | asm!("{a}", a = const foo, a = const bar);
| ^^^ non-constant value
Expand All @@ -409,7 +409,7 @@ error[E0435]: attempt to use a non-constant value in a constant
--> $DIR/parse-error.rs:55:46
|
LL | let mut bar = 0;
| ---------- help: consider using `const` instead of `let`: `const bar`
| ----------- help: consider using `const` instead of `let`: `const bar`
...
LL | asm!("{a}", a = const foo, a = const bar);
| ^^^ non-constant value
Expand All @@ -418,7 +418,7 @@ error[E0435]: attempt to use a non-constant value in a constant
--> $DIR/parse-error.rs:62:45
|
LL | let mut bar = 0;
| ---------- help: consider using `const` instead of `let`: `const bar`
| ----------- help: consider using `const` instead of `let`: `const bar`
...
LL | asm!("{a}", in("x0") foo, a = const bar);
| ^^^ non-constant value
Expand All @@ -427,7 +427,7 @@ error[E0435]: attempt to use a non-constant value in a constant
--> $DIR/parse-error.rs:65:45
|
LL | let mut bar = 0;
| ---------- help: consider using `const` instead of `let`: `const bar`
| ----------- help: consider using `const` instead of `let`: `const bar`
...
LL | asm!("{a}", in("x0") foo, a = const bar);
| ^^^ non-constant value
Expand All @@ -436,7 +436,7 @@ error[E0435]: attempt to use a non-constant value in a constant
--> $DIR/parse-error.rs:68:41
|
LL | let mut bar = 0;
| ---------- help: consider using `const` instead of `let`: `const bar`
| ----------- help: consider using `const` instead of `let`: `const bar`
...
LL | asm!("{1}", in("x0") foo, const bar);
| ^^^ non-constant value
Expand Down
14 changes: 7 additions & 7 deletions src/test/ui/asm/x86_64/parse-error.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,7 @@ error[E0435]: attempt to use a non-constant value in a constant
--> $DIR/parse-error.rs:39:37
|
LL | let mut foo = 0;
| ---------- help: consider using `const` instead of `let`: `const foo`
| ----------- help: consider using `const` instead of `let`: `const foo`
...
LL | asm!("{}", options(), const foo);
| ^^^ non-constant value
Expand All @@ -403,7 +403,7 @@ error[E0435]: attempt to use a non-constant value in a constant
--> $DIR/parse-error.rs:50:44
|
LL | let mut foo = 0;
| ---------- help: consider using `const` instead of `let`: `const foo`
| ----------- help: consider using `const` instead of `let`: `const foo`
...
LL | asm!("{}", clobber_abi("C"), const foo);
| ^^^ non-constant value
Expand All @@ -412,7 +412,7 @@ error[E0435]: attempt to use a non-constant value in a constant
--> $DIR/parse-error.rs:57:31
|
LL | let mut foo = 0;
| ---------- help: consider using `const` instead of `let`: `const foo`
| ----------- help: consider using `const` instead of `let`: `const foo`
...
LL | asm!("{a}", a = const foo, a = const bar);
| ^^^ non-constant value
Expand All @@ -421,7 +421,7 @@ error[E0435]: attempt to use a non-constant value in a constant
--> $DIR/parse-error.rs:57:46
|
LL | let mut bar = 0;
| ---------- help: consider using `const` instead of `let`: `const bar`
| ----------- help: consider using `const` instead of `let`: `const bar`
...
LL | asm!("{a}", a = const foo, a = const bar);
| ^^^ non-constant value
Expand All @@ -430,7 +430,7 @@ error[E0435]: attempt to use a non-constant value in a constant
--> $DIR/parse-error.rs:64:46
|
LL | let mut bar = 0;
| ---------- help: consider using `const` instead of `let`: `const bar`
| ----------- help: consider using `const` instead of `let`: `const bar`
...
LL | asm!("{a}", in("eax") foo, a = const bar);
| ^^^ non-constant value
Expand All @@ -439,7 +439,7 @@ error[E0435]: attempt to use a non-constant value in a constant
--> $DIR/parse-error.rs:67:46
|
LL | let mut bar = 0;
| ---------- help: consider using `const` instead of `let`: `const bar`
| ----------- help: consider using `const` instead of `let`: `const bar`
...
LL | asm!("{a}", in("eax") foo, a = const bar);
| ^^^ non-constant value
Expand All @@ -448,7 +448,7 @@ error[E0435]: attempt to use a non-constant value in a constant
--> $DIR/parse-error.rs:70:42
|
LL | let mut bar = 0;
| ---------- help: consider using `const` instead of `let`: `const bar`
| ----------- help: consider using `const` instead of `let`: `const bar`
...
LL | asm!("{1}", in("eax") foo, const bar);
| ^^^ non-constant value
Expand Down
21 changes: 21 additions & 0 deletions src/test/ui/consts/issue-91560.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// Regression test for issue #91560.

// run-rustfix

#![allow(unused,non_upper_case_globals)]

fn foo() {
const length: usize = 2;
//~^ HELP: consider using `const`
let arr = [0; length];
//~^ ERROR: attempt to use a non-constant value in a constant [E0435]
}

fn bar() {
const length: usize = 2;
//~^ HELP: consider using `const`
let arr = [0; length];
//~^ ERROR: attempt to use a non-constant value in a constant [E0435]
}

fn main() {}
21 changes: 21 additions & 0 deletions src/test/ui/consts/issue-91560.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// Regression test for issue #91560.

// run-rustfix

#![allow(unused,non_upper_case_globals)]

fn foo() {
let mut length: usize = 2;
//~^ HELP: consider using `const`
let arr = [0; length];
//~^ ERROR: attempt to use a non-constant value in a constant [E0435]
}

fn bar() {
let length: usize = 2;
//~^ HELP: consider using `const`
let arr = [0; length];
//~^ ERROR: attempt to use a non-constant value in a constant [E0435]
}

fn main() {}
21 changes: 21 additions & 0 deletions src/test/ui/consts/issue-91560.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
error[E0435]: attempt to use a non-constant value in a constant
--> $DIR/issue-91560.rs:10:19
|
LL | let mut length: usize = 2;
| -------------- help: consider using `const` instead of `let`: `const length`
LL |
LL | let arr = [0; length];
| ^^^^^^ non-constant value

error[E0435]: attempt to use a non-constant value in a constant
--> $DIR/issue-91560.rs:17:19
|
LL | let length: usize = 2;
| ------------ help: consider using `const` instead of `let`: `const length`
LL |
LL | let arr = [0; length];
| ^^^^^^ non-constant value

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0435`.

0 comments on commit 9bbd8ec

Please sign in to comment.