From 12545c75ff9c9ad72ad50bbbb3cc1f5c04c8037a Mon Sep 17 00:00:00 2001 From: Laurent Bonnans Date: Sun, 5 Jan 2020 00:58:41 +0100 Subject: [PATCH] Handle multiple error fix suggestions carefuly The existing code seems to assume that substitutions spans are disjoint, which is not always the case. In the example: pub trait AAAA {} pub trait B {} pub trait C {} pub type T = P; , we get three substituions starting from ':' and ending respectively at the end of each trait token. With the former offset calculation, this would cause `underline_start` to eventually become negative before being converted to `usize`... The new version may report erroneous results for non perfectly overlapping substitutions but I don't know if such examples exist. Alternatively, we could detect these cases and trim out overlapping substitutions. --- src/librustc_errors/emitter.rs | 18 +++++++++++++----- ...-67690-type-alias-bound-diagnostic-crash.rs | 8 ++++++++ ...90-type-alias-bound-diagnostic-crash.stderr | 12 ++++++++++++ src/test/ui/type/type-alias-bounds.rs | 2 +- src/test/ui/type/type-alias-bounds.stderr | 6 +++--- 5 files changed, 37 insertions(+), 9 deletions(-) create mode 100644 src/test/ui/type/issue-67690-type-alias-bound-diagnostic-crash.rs create mode 100644 src/test/ui/type/issue-67690-type-alias-bound-diagnostic-crash.stderr diff --git a/src/librustc_errors/emitter.rs b/src/librustc_errors/emitter.rs index 541b89e6fce5f..526b4e2971bef 100644 --- a/src/librustc_errors/emitter.rs +++ b/src/librustc_errors/emitter.rs @@ -1530,7 +1530,7 @@ impl EmitterWriter { // This offset and the ones below need to be signed to account for replacement code // that is shorter than the original code. - let mut offset: isize = 0; + let mut offsets: Vec<(usize, isize)> = Vec::new(); // Only show an underline in the suggestions if the suggestion is not the // entirety of the code being shown and the displayed code is not multiline. if show_underline { @@ -1550,12 +1550,19 @@ impl EmitterWriter { .map(|ch| unicode_width::UnicodeWidthChar::width(ch).unwrap_or(1)) .sum(); + let offset: isize = offsets + .iter() + .filter_map( + |(start, v)| if span_start_pos <= *start { None } else { Some(v) }, + ) + .sum(); let underline_start = (span_start_pos + start) as isize + offset; let underline_end = (span_start_pos + start + sub_len) as isize + offset; + assert!(underline_start >= 0 && underline_end >= 0); for p in underline_start..underline_end { buffer.putc( row_num, - max_line_num_len + 3 + p as usize, + ((max_line_num_len + 3) as isize + p) as usize, '^', Style::UnderlinePrimary, ); @@ -1565,7 +1572,7 @@ impl EmitterWriter { for p in underline_start - 1..underline_start + 1 { buffer.putc( row_num, - max_line_num_len + 3 + p as usize, + ((max_line_num_len + 3) as isize + p) as usize, '-', Style::UnderlineSecondary, ); @@ -1582,8 +1589,9 @@ impl EmitterWriter { // length of the code to be substituted let snippet_len = span_end_pos as isize - span_start_pos as isize; // For multiple substitutions, use the position *after* the previous - // substitutions have happened. - offset += full_sub_len - snippet_len; + // substitutions have happened, only when further substitutions are + // located strictly after. + offsets.push((span_end_pos, full_sub_len - snippet_len)); } row_num += 1; } diff --git a/src/test/ui/type/issue-67690-type-alias-bound-diagnostic-crash.rs b/src/test/ui/type/issue-67690-type-alias-bound-diagnostic-crash.rs new file mode 100644 index 0000000000000..68aadcf605384 --- /dev/null +++ b/src/test/ui/type/issue-67690-type-alias-bound-diagnostic-crash.rs @@ -0,0 +1,8 @@ +// Regression test for issue #67690 +// Rustc endless loop out-of-memory and consequent SIGKILL in generic new type + +// check-pass +pub type T = P; +//~^ WARN bounds on generic parameters are not enforced in type aliases + +fn main() {} diff --git a/src/test/ui/type/issue-67690-type-alias-bound-diagnostic-crash.stderr b/src/test/ui/type/issue-67690-type-alias-bound-diagnostic-crash.stderr new file mode 100644 index 0000000000000..37b51b50b964e --- /dev/null +++ b/src/test/ui/type/issue-67690-type-alias-bound-diagnostic-crash.stderr @@ -0,0 +1,12 @@ +warning: bounds on generic parameters are not enforced in type aliases + --> $DIR/issue-67690-type-alias-bound-diagnostic-crash.rs:5:15 + | +LL | pub type T = P; + | ^^^^ ^^^^ ^^^^ + | + = note: `#[warn(type_alias_bounds)]` on by default +help: the bound will not be checked when the type alias is used, and should be removed + | +LL | pub type T

= P; + | -- + diff --git a/src/test/ui/type/type-alias-bounds.rs b/src/test/ui/type/type-alias-bounds.rs index 06705a2ebf5dd..65b79650d4d72 100644 --- a/src/test/ui/type/type-alias-bounds.rs +++ b/src/test/ui/type/type-alias-bounds.rs @@ -1,6 +1,6 @@ // Test `ignored_generic_bounds` lint warning about bounds in type aliases. -// build-pass (FIXME(62277): could be check-pass?) +// check-pass #![allow(dead_code)] use std::rc::Rc; diff --git a/src/test/ui/type/type-alias-bounds.stderr b/src/test/ui/type/type-alias-bounds.stderr index c381d30c64f14..e4d3753f8a59a 100644 --- a/src/test/ui/type/type-alias-bounds.stderr +++ b/src/test/ui/type/type-alias-bounds.stderr @@ -8,7 +8,7 @@ LL | type SVec = Vec; help: the bound will not be checked when the type alias is used, and should be removed | LL | type SVec = Vec; - | -- -- + | -- warning: where clauses are not enforced in type aliases --> $DIR/type-alias-bounds.rs:10:21 @@ -30,7 +30,7 @@ LL | type VVec<'b, 'a: 'b + 'b> = (&'b u32, Vec<&'a i32>); help: the bound will not be checked when the type alias is used, and should be removed | LL | type VVec<'b, 'a> = (&'b u32, Vec<&'a i32>); - | -- -- + | -- warning: bounds on generic parameters are not enforced in type aliases --> $DIR/type-alias-bounds.rs:14:18 @@ -41,7 +41,7 @@ LL | type WVec<'b, T: 'b + 'b> = (&'b u32, Vec); help: the bound will not be checked when the type alias is used, and should be removed | LL | type WVec<'b, T> = (&'b u32, Vec); - | -- -- + | -- warning: where clauses are not enforced in type aliases --> $DIR/type-alias-bounds.rs:16:25