Skip to content

typeck: fix ? suggestion span #123654

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

Merged
merged 3 commits into from
Apr 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 3 additions & 12 deletions compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1291,12 +1291,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())]
Expand Down Expand Up @@ -1901,12 +1896,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;
}

Expand Down
39 changes: 39 additions & 0 deletions compiler/rustc_span/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
37 changes: 0 additions & 37 deletions tests/ui/typeck/issue-112007-leaked-writeln-macro-internals.fixed

This file was deleted.

18 changes: 13 additions & 5 deletions tests/ui/typeck/issue-112007-leaked-writeln-macro-internals.rs
Original file line number Diff line number Diff line change
@@ -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 {
Expand All @@ -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 {
Expand All @@ -34,4 +42,4 @@ fn foo<W: std::fmt::Write>(w: &mut W) -> std::fmt::Result {
Ok(())
}

fn main() {}
pub fn main() {}
12 changes: 10 additions & 2 deletions tests/ui/typeck/issue-112007-leaked-writeln-macro-internals.stderr
Original file line number Diff line number Diff line change
@@ -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 ->")?;
Expand All @@ -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 ->")?;
Expand All @@ -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

Expand Down
22 changes: 22 additions & 0 deletions tests/ui/typeck/question-mark-operator-suggestion-span.rs
Original file line number Diff line number Diff line change
@@ -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(())
}
31 changes: 31 additions & 0 deletions tests/ui/typeck/question-mark-operator-suggestion-span.stderr
Original file line number Diff line number Diff line change
@@ -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`.
Loading