diff options
7 files changed, 118 insertions, 56 deletions
diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs index 804cd033538..ce203eae95f 100644 --- a/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs +++ b/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs @@ -1287,12 +1287,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { ty::TraitRef::new(self.tcx, into_def_id, [expr_ty, expected_ty]), )) { - let mut span = expr.span; - while expr.span.eq_ctxt(span) - && let Some(parent_callsite) = span.parent_callsite() - { - span = parent_callsite; - } + let span = expr.span.find_oldest_ancestor_in_same_ctxt(); let mut sugg = if expr.precedence().order() >= PREC_POSTFIX { vec![(span.shrink_to_hi(), ".into()".to_owned())] @@ -1897,12 +1892,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { None => sugg.to_string(), }; - err.span_suggestion_verbose( - expr.span.shrink_to_hi(), - msg, - sugg, - Applicability::HasPlaceholders, - ); + let span = expr.span.find_oldest_ancestor_in_same_ctxt(); + err.span_suggestion_verbose(span.shrink_to_hi(), msg, sugg, Applicability::HasPlaceholders); return true; } diff --git a/compiler/rustc_span/src/lib.rs b/compiler/rustc_span/src/lib.rs index 7ce879807ca..c1e1175b4bd 100644 --- a/compiler/rustc_span/src/lib.rs +++ b/compiler/rustc_span/src/lib.rs @@ -743,6 +743,45 @@ impl Span { Some(self) } + /// Recursively walk down the expansion ancestors to find the oldest ancestor span with the same + /// [`SyntaxContext`] the initial span. + /// + /// This method is suitable for peeling through *local* macro expansions to find the "innermost" + /// span that is still local and shares the same [`SyntaxContext`]. For example, given + /// + /// ```ignore (illustrative example, contains type error) + /// macro_rules! outer { + /// ($x: expr) => { + /// inner!($x) + /// } + /// } + /// + /// macro_rules! inner { + /// ($x: expr) => { + /// format!("error: {}", $x) + /// //~^ ERROR mismatched types + /// } + /// } + /// + /// fn bar(x: &str) -> Result<(), Box<dyn std::error::Error>> { + /// Err(outer!(x)) + /// } + /// ``` + /// + /// if provided the initial span of `outer!(x)` inside `bar`, this method will recurse + /// the parent callsites until we reach `format!("error: {}", $x)`, at which point it is the + /// oldest ancestor span that is both still local and shares the same [`SyntaxContext`] as the + /// initial span. + pub fn find_oldest_ancestor_in_same_ctxt(self) -> Span { + let mut cur = self; + while cur.eq_ctxt(self) + && let Some(parent_callsite) = cur.parent_callsite() + { + cur = parent_callsite; + } + cur + } + /// Edition of the crate from which this span came. pub fn edition(self) -> edition::Edition { self.ctxt().edition() diff --git a/tests/ui/typeck/issue-112007-leaked-writeln-macro-internals.fixed b/tests/ui/typeck/issue-112007-leaked-writeln-macro-internals.fixed deleted file mode 100644 index dcb256de18f..00000000000 --- a/tests/ui/typeck/issue-112007-leaked-writeln-macro-internals.fixed +++ /dev/null @@ -1,37 +0,0 @@ -//@ run-rustfix -#![allow(dead_code)] - -// https://github.com/rust-lang/rust/issues/112007 -fn bug_report<W: std::fmt::Write>(w: &mut W) -> std::fmt::Result { - if true { - writeln!(w, "`;?` here ->")?; - } else { - return writeln!(w, "but not here"); - //~^ ERROR mismatched types - }; - Ok(()) -} - -macro_rules! baz { - ($w: expr) => { - bar!($w) - } -} - -macro_rules! bar { - ($w: expr) => { - writeln!($w, "but not here") - //~^ ERROR mismatched types - } -} - -fn foo<W: std::fmt::Write>(w: &mut W) -> std::fmt::Result { - if true { - writeln!(w, "`;?` here ->")?; - } else { - return baz!(w); - }; - Ok(()) -} - -fn main() {} diff --git a/tests/ui/typeck/issue-112007-leaked-writeln-macro-internals.rs b/tests/ui/typeck/issue-112007-leaked-writeln-macro-internals.rs index 58cd6cbf20c..7ec9f0d4cdb 100644 --- a/tests/ui/typeck/issue-112007-leaked-writeln-macro-internals.rs +++ b/tests/ui/typeck/issue-112007-leaked-writeln-macro-internals.rs @@ -1,8 +1,16 @@ -//@ run-rustfix -#![allow(dead_code)] +// Check that we don't leak stdlib implementation details through suggestions. +// Also check that the suggestion provided tries as hard as it can to see through local macros. +// +// FIXME(jieyouxu): this test is NOT run-rustfix because this test contains conflicting +// MaybeIncorrect suggestions: +// +// 1. `return ... ;` +// 2. `?` +// +// when the suggestions are applied to the same file, it becomes uncompilable. // https://github.com/rust-lang/rust/issues/112007 -fn bug_report<W: std::fmt::Write>(w: &mut W) -> std::fmt::Result { +pub fn bug_report<W: std::fmt::Write>(w: &mut W) -> std::fmt::Result { if true { writeln!(w, "`;?` here ->")?; } else { @@ -25,7 +33,7 @@ macro_rules! bar { } } -fn foo<W: std::fmt::Write>(w: &mut W) -> std::fmt::Result { +pub fn foo<W: std::fmt::Write>(w: &mut W) -> std::fmt::Result { if true { writeln!(w, "`;?` here ->")?; } else { @@ -34,4 +42,4 @@ fn foo<W: std::fmt::Write>(w: &mut W) -> std::fmt::Result { Ok(()) } -fn main() {} +pub fn main() {} diff --git a/tests/ui/typeck/issue-112007-leaked-writeln-macro-internals.stderr b/tests/ui/typeck/issue-112007-leaked-writeln-macro-internals.stderr index df2e06e8f3b..889d2c94d0c 100644 --- a/tests/ui/typeck/issue-112007-leaked-writeln-macro-internals.stderr +++ b/tests/ui/typeck/issue-112007-leaked-writeln-macro-internals.stderr @@ -1,5 +1,5 @@ error[E0308]: mismatched types - --> $DIR/issue-112007-leaked-writeln-macro-internals.rs:9:9 + --> $DIR/issue-112007-leaked-writeln-macro-internals.rs:17:9 | LL | / if true { LL | | writeln!(w, "`;?` here ->")?; @@ -21,9 +21,13 @@ help: you might have meant to return this value | LL | return writeln!(w, "but not here"); | ++++++ + +help: use the `?` operator to extract the `Result<(), std::fmt::Error>` value, propagating a `Result::Err` value to the caller + | +LL | writeln!(w, "but not here")? + | + error[E0308]: mismatched types - --> $DIR/issue-112007-leaked-writeln-macro-internals.rs:32:9 + --> $DIR/issue-112007-leaked-writeln-macro-internals.rs:40:9 | LL | / if true { LL | | writeln!(w, "`;?` here ->")?; @@ -44,6 +48,10 @@ help: you might have meant to return this value | LL | return baz!(w); | ++++++ + +help: use the `?` operator to extract the `Result<(), std::fmt::Error>` value, propagating a `Result::Err` value to the caller + | +LL | writeln!($w, "but not here")? + | + error: aborting due to 2 previous errors diff --git a/tests/ui/typeck/question-mark-operator-suggestion-span.rs b/tests/ui/typeck/question-mark-operator-suggestion-span.rs new file mode 100644 index 00000000000..7aea6e63dd1 --- /dev/null +++ b/tests/ui/typeck/question-mark-operator-suggestion-span.rs @@ -0,0 +1,22 @@ +// Check that we don't construct a span for `?` suggestions that point into non-local macros +// like into the stdlib where the user has no control over. +// +// FIXME(jieyouxu): this test is currently NOT run-rustfix because there are conflicting +// MaybeIncorrect suggestions: +// +// 1. adding `return ... ;`, and +// 2. adding `?`. +// +// When rustfix puts those together, the fixed file now contains uncompilable code. + +#![crate_type = "lib"] + +pub fn bug_report<W: std::fmt::Write>(w: &mut W) -> std::fmt::Result { + if true { + writeln!(w, "`;?` here ->")?; + } else { + writeln!(w, "but not here") + //~^ ERROR mismatched types + } + Ok(()) +} diff --git a/tests/ui/typeck/question-mark-operator-suggestion-span.stderr b/tests/ui/typeck/question-mark-operator-suggestion-span.stderr new file mode 100644 index 00000000000..089b3bcd198 --- /dev/null +++ b/tests/ui/typeck/question-mark-operator-suggestion-span.stderr @@ -0,0 +1,31 @@ +error[E0308]: mismatched types + --> $DIR/question-mark-operator-suggestion-span.rs:18:9 + | +LL | / if true { +LL | | writeln!(w, "`;?` here ->")?; +LL | | } else { +LL | | writeln!(w, "but not here") + | | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `()`, found `Result<(), Error>` +LL | | +LL | | } + | |_____- expected this to be `()` + | + = note: expected unit type `()` + found enum `Result<(), std::fmt::Error>` + = note: this error originates in the macro `writeln` (in Nightly builds, run with -Z macro-backtrace for more info) +help: consider using a semicolon here + | +LL | }; + | + +help: you might have meant to return this value + | +LL | return writeln!(w, "but not here"); + | ++++++ + +help: use the `?` operator to extract the `Result<(), std::fmt::Error>` value, propagating a `Result::Err` value to the caller + | +LL | writeln!(w, "but not here")? + | + + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0308`. |