Skip to content
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

Avoid wrong code suggesting for attribute macro #107254

Merged
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
3 changes: 3 additions & 0 deletions compiler/rustc_hir_typeck/src/demand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -880,6 +880,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
expected: Ty<'tcx>,
expr_ty: Ty<'tcx>,
) -> bool {
if in_external_macro(self.tcx.sess, expr.span) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@compiler-errors
My change at here makes this case failed:

rustc --edition 2021 tests/ui/async-await/proper-span-for-type-error.rs
error[E0308]: mismatched types
 --> tests/ui/async-await/proper-span-for-type-error.rs:8:5
  |
8 |     a().await //~ ERROR mismatched types
  |     ^^^^^^^^^ expected enum `Result`, found `()`
  |
  = note:   expected enum `Result<(), i32>`
          found unit type `()`
help: try adding an expression at the end of the block
  |
8 ~     a().await;
9 ~     Ok(()) //~ ERROR mismatched types
  |

This is because in_external_macro is not right, missing DesugaringKind::Await, so I added it and with extra DesugaringKind::Async.

So the new suggestion comes out:

_consume_reference::<i32>(&*async { Box::new(7_i32) }.await);

It seems right, so I keep it.

By the way, I think our current in_external_macro still missing some DesugaringKind, and the in_external_macro is not a proper name right now considering it actually not just checking Macro, a suitable function name should be something like proper_for_suggesting.

And we have a similar function in span, but it definitely need some fix:

pub fn can_be_used_for_suggestions(self) -> bool {
!self.from_expansion()
// FIXME: If this span comes from a `derive` macro but it points at code the user wrote,
// the callsite span and the span will be pointing at different places. It also means that
// we can safely provide suggestions on this span.
|| (matches!(self.ctxt().outer_expn_data().kind, ExpnKind::Macro(MacroKind::Derive, _))
&& self.parent_callsite().map(|p| (p.lo(), p.hi())) != Some((self.lo(), self.hi())))
}

We need some cleanup for these functions here?

return false;
}
if let ty::Adt(expected_adt, args) = expected.kind() {
if let hir::ExprKind::Field(base, ident) = expr.kind {
let base_ty = self.typeck_results.borrow().expr_ty(base);
Expand Down
15 changes: 14 additions & 1 deletion compiler/rustc_middle/src/lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -449,7 +449,11 @@ pub fn in_external_macro(sess: &Session, span: Span) -> bool {
match expn_data.kind {
ExpnKind::Root
| ExpnKind::Desugaring(
DesugaringKind::ForLoop | DesugaringKind::WhileLoop | DesugaringKind::OpaqueTy,
DesugaringKind::ForLoop
| DesugaringKind::WhileLoop
| DesugaringKind::OpaqueTy
| DesugaringKind::Async
| DesugaringKind::Await,
) => false,
ExpnKind::AstPass(_) | ExpnKind::Desugaring(_) => true, // well, it's "external"
ExpnKind::Macro(MacroKind::Bang, _) => {
Expand All @@ -459,3 +463,12 @@ pub fn in_external_macro(sess: &Session, span: Span) -> bool {
ExpnKind::Macro { .. } => true, // definitely a plugin
}
}

/// Return whether `span` is generated by `async` or `await`.
pub fn is_from_async_await(span: Span) -> bool {
let expn_data = span.ctxt().outer_expn_data();
match expn_data.kind {
ExpnKind::Desugaring(DesugaringKind::Async | DesugaringKind::Await) => true,
_ => false,
}
}
16 changes: 13 additions & 3 deletions src/tools/clippy/clippy_lints/src/redundant_locals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@ use clippy_utils::diagnostics::span_lint_and_help;
use clippy_utils::is_from_proc_macro;
use clippy_utils::ty::needs_ordered_drop;
use rustc_hir::def::Res;
use rustc_hir::{BindingAnnotation, ByRef, Expr, ExprKind, HirId, Local, Node, Pat, PatKind, QPath};
use rustc_hir::{
BindingAnnotation, ByRef, Expr, ExprKind, HirId, Local, Node, Pat, PatKind, QPath,
};
use rustc_lint::{LateContext, LateLintPass, LintContext};
use rustc_middle::lint::in_external_macro;
use rustc_middle::lint::{in_external_macro, is_from_async_await};
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::symbol::Ident;

Expand Down Expand Up @@ -65,6 +67,9 @@ impl<'tcx> LateLintPass<'tcx> for RedundantLocals {
// the local is user-controlled
if !in_external_macro(cx.sess(), local.span);
if !is_from_proc_macro(cx, expr);
// Async function parameters are lowered into the closure body, so we can't lint them.
// see `lower_maybe_async_body` in `rust_ast_lowering`
if !is_from_async_await(local.span);
then {
span_lint_and_help(
cx,
Expand Down Expand Up @@ -93,7 +98,12 @@ fn find_binding(pat: &Pat<'_>, name: Ident) -> Option<BindingAnnotation> {
}

/// Check if a rebinding of a local affects the code's drop behavior.
fn affects_drop_behavior<'tcx>(cx: &LateContext<'tcx>, bind: HirId, rebind: HirId, rebind_expr: &Expr<'tcx>) -> bool {
fn affects_drop_behavior<'tcx>(
cx: &LateContext<'tcx>,
bind: HirId,
rebind: HirId,
rebind_expr: &Expr<'tcx>,
) -> bool {
let hir = cx.tcx.hir();

// the rebinding is in a different scope than the original binding
Expand Down
13 changes: 13 additions & 0 deletions tests/ui/coercion/coerce-block-tail-83783.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// run-rustfix
// edition:2018
fn _consume_reference<T: ?Sized>(_: &T) {}

async fn _foo() {
_consume_reference::<i32>(&Box::new(7_i32));
_consume_reference::<i32>(&*async { Box::new(7_i32) }.await);
//~^ ERROR mismatched types
_consume_reference::<[i32]>(&vec![7_i32]);
_consume_reference::<[i32]>(&async { vec![7_i32] }.await);
}

fn main() { }
2 changes: 1 addition & 1 deletion tests/ui/coercion/coerce-block-tail-83783.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// check-fail
// run-rustfix
// edition:2018
fn _consume_reference<T: ?Sized>(_: &T) {}

Expand Down
4 changes: 4 additions & 0 deletions tests/ui/coercion/coerce-block-tail-83783.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ LL | _consume_reference::<i32>(&async { Box::new(7_i32) }.await);
|
= note: expected type `i32`
found struct `Box<i32>`
help: consider unboxing the value
|
LL | _consume_reference::<i32>(&*async { Box::new(7_i32) }.await);
| +

error: aborting due to previous error

Expand Down
13 changes: 13 additions & 0 deletions tests/ui/proc-macro/auxiliary/issue-107113.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// force-host
// no-prefer-dynamic

#![crate_type = "proc-macro"]

extern crate proc_macro;

use proc_macro::TokenStream;

#[proc_macro_attribute]
pub fn main(_: TokenStream, item: TokenStream) -> TokenStream {
"fn main() -> std::io::Result<()> { () } ".parse().unwrap()
}
8 changes: 8 additions & 0 deletions tests/ui/proc-macro/issue-107113-wrap.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// edition:2021
// aux-build:issue-107113.rs

#[macro_use]
extern crate issue_107113;

#[issue_107113::main] //~ ERROR mismatched types [E0308]
async fn main() -> std::io::Result<()> {}
16 changes: 16 additions & 0 deletions tests/ui/proc-macro/issue-107113-wrap.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
error[E0308]: mismatched types
--> $DIR/issue-107113-wrap.rs:7:1
|
LL | #[issue_107113::main]
| ^^^^^^^^^^^^^^^^^^^^^
| |
| expected `Result<(), Error>`, found `()`
| expected `Result<(), std::io::Error>` because of return type
|
= note: expected enum `Result<(), std::io::Error>`
found unit type `()`
= note: this error originates in the attribute macro `issue_107113::main` (in Nightly builds, run with -Z macro-backtrace for more info)

error: aborting due to previous error

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