Rollup of 11 pull requests

compiler/rustc_codegen_ssa/src/mir/analyze.rs

@@ -199,7 +199,7 @@ impl<Bx: BuilderMethods<'a, 'tcx>> LocalAnalyzer<'mir, 'a, 'tcx, Bx> {
             }
 
             self.visit_local(&place_ref.local, context, location);
-            self.visit_projection(place_ref.local, place_ref.projection, context, location);
+            self.visit_projection(*place_ref, context, location);
         }
     }
 }

compiler/rustc_hir/src/hir.rs

@@ -1,5 +1,5 @@
 // ignore-tidy-filelength
-use crate::def::{DefKind, Namespace, Res};
+use crate::def::{CtorKind, DefKind, Namespace, Res};
 use crate::def_id::DefId;
 crate use crate::hir_id::HirId;
 use crate::{itemlikevisit, LangItem};
@@ -1576,6 +1576,63 @@ impl Expr<'_> {
         }
         expr
     }
+
+    pub fn can_have_side_effects(&self) -> bool {
+        match self.peel_drop_temps().kind {
+            ExprKind::Path(_) | ExprKind::Lit(_) => false,
+            ExprKind::Type(base, _)
+            | ExprKind::Unary(_, base)
+            | ExprKind::Field(base, _)
+            | ExprKind::Index(base, _)
+            | ExprKind::AddrOf(.., base)
+            | ExprKind::Cast(base, _) => {
+                // This isn't exactly true for `Index` and all `Unnary`, but we are using this
+                // method exclusively for diagnostics and there's a *cultural* pressure against
+                // them being used only for its side-effects.
+                base.can_have_side_effects()
+            }
+            ExprKind::Struct(_, fields, init) => fields
+                .iter()
+                .map(|field| field.expr)
+                .chain(init.into_iter())
+                .all(|e| e.can_have_side_effects()),
+
+            ExprKind::Array(args)
+            | ExprKind::Tup(args)
+            | ExprKind::Call(
+                Expr {
+                    kind:
+                        ExprKind::Path(QPath::Resolved(
+                            None,
+                            Path { res: Res::Def(DefKind::Ctor(_, CtorKind::Fn), _), .. },
+                        )),
+                    ..
+                },
+                args,
+            ) => args.iter().all(|arg| arg.can_have_side_effects()),
+            ExprKind::If(..)
+            | ExprKind::Match(..)
+            | ExprKind::MethodCall(..)
+            | ExprKind::Call(..)
+            | ExprKind::Closure(..)
+            | ExprKind::Block(..)
+            | ExprKind::Repeat(..)
+            | ExprKind::Break(..)
+            | ExprKind::Continue(..)
+            | ExprKind::Ret(..)
+            | ExprKind::Loop(..)
+            | ExprKind::Assign(..)
+            | ExprKind::InlineAsm(..)
+            | ExprKind::LlvmInlineAsm(..)
+            | ExprKind::AssignOp(..)
+            | ExprKind::ConstBlock(..)
+            | ExprKind::Box(..)
+            | ExprKind::Binary(..)
+            | ExprKind::Yield(..)
+            | ExprKind::DropTemps(..)
+            | ExprKind::Err => true,
+        }
+    }
 }
 
 /// Checks if the specified expression is a built-in range literal.

compiler/rustc_middle/src/mir/visit.rs

@@ -998,12 +998,11 @@ macro_rules! visit_place_fns {
     () => {
         fn visit_projection(
             &mut self,
-            local: Local,
-            projection: &[PlaceElem<'tcx>],
+            place_ref: PlaceRef<'tcx>,
             context: PlaceContext,
             location: Location,
         ) {
-            self.super_projection(local, projection, context, location);
+            self.super_projection(place_ref, context, location);
         }
 
         fn visit_projection_elem(
@@ -1033,20 +1032,20 @@ macro_rules! visit_place_fns {
 
             self.visit_local(&place.local, context, location);
 
-            self.visit_projection(place.local, &place.projection, context, location);
+            self.visit_projection(place.as_ref(), context, location);
         }
 
         fn super_projection(
             &mut self,
-            local: Local,
-            projection: &[PlaceElem<'tcx>],
+            place_ref: PlaceRef<'tcx>,
             context: PlaceContext,
             location: Location,
         ) {
-            let mut cursor = projection;
+            // FIXME: Use PlaceRef::iter_projections, once that exists.
+            let mut cursor = place_ref.projection;
             while let &[ref proj_base @ .., elem] = cursor {
                 cursor = proj_base;
-                self.visit_projection_elem(local, cursor, elem, context, location);
+                self.visit_projection_elem(place_ref.local, cursor, elem, context, location);
             }
         }
 

compiler/rustc_mir/src/dataflow/impls/liveness.rs

@@ -95,7 +95,7 @@ where
 
         // We purposefully do not call `super_place` here to avoid calling `visit_local` for this
         // place with one of the `Projection` variants of `PlaceContext`.
-        self.visit_projection(local, projection, context, location);
+        self.visit_projection(place.as_ref(), context, location);
 
         match DefUse::for_place(context) {
             // Treat derefs as a use of the base local. `*p = 4` is not a def of `p` but a use.

compiler/rustc_mir/src/transform/check_consts/validation.rs

@@ -515,7 +515,7 @@ impl Visitor<'tcx> for Validator<'mir, 'tcx> {
         // Special-case reborrows to be more like a copy of a reference.
         match *rvalue {
             Rvalue::Ref(_, kind, place) => {
-                if let Some(reborrowed_proj) = place_as_reborrow(self.tcx, self.body, place) {
+                if let Some(reborrowed_place_ref) = place_as_reborrow(self.tcx, self.body, place) {
                     let ctx = match kind {
                         BorrowKind::Shared => {
                             PlaceContext::NonMutatingUse(NonMutatingUseContext::SharedBorrow)
@@ -530,21 +530,21 @@ impl Visitor<'tcx> for Validator<'mir, 'tcx> {
                             PlaceContext::MutatingUse(MutatingUseContext::Borrow)
                         }
                     };
-                    self.visit_local(&place.local, ctx, location);
-                    self.visit_projection(place.local, reborrowed_proj, ctx, location);
+                    self.visit_local(&reborrowed_place_ref.local, ctx, location);
+                    self.visit_projection(reborrowed_place_ref, ctx, location);
                     return;
                 }
             }
             Rvalue::AddressOf(mutbl, place) => {
-                if let Some(reborrowed_proj) = place_as_reborrow(self.tcx, self.body, place) {
+                if let Some(reborrowed_place_ref) = place_as_reborrow(self.tcx, self.body, place) {
                     let ctx = match mutbl {
                         Mutability::Not => {
                             PlaceContext::NonMutatingUse(NonMutatingUseContext::AddressOf)
                         }
                         Mutability::Mut => PlaceContext::MutatingUse(MutatingUseContext::AddressOf),
                     };
-                    self.visit_local(&place.local, ctx, location);
-                    self.visit_projection(place.local, reborrowed_proj, ctx, location);
+                    self.visit_local(&reborrowed_place_ref.local, ctx, location);
+                    self.visit_projection(reborrowed_place_ref, ctx, location);
                     return;
                 }
             }
@@ -1039,7 +1039,7 @@ fn place_as_reborrow(
     tcx: TyCtxt<'tcx>,
     body: &Body<'tcx>,
     place: Place<'tcx>,
-) -> Option<&'a [PlaceElem<'tcx>]> {
+) -> Option<PlaceRef<'tcx>> {
     match place.as_ref().last_projection() {
         Some((place_base, ProjectionElem::Deref)) => {
             // A borrow of a `static` also looks like `&(*_1)` in the MIR, but `_1` is a `const`
@@ -1048,13 +1048,14 @@ fn place_as_reborrow(
                 None
             } else {
                 // Ensure the type being derefed is a reference and not a raw pointer.
-                //
                 // This is sufficient to prevent an access to a `static mut` from being marked as a
                 // reborrow, even if the check above were to disappear.
                 let inner_ty = place_base.ty(body, tcx).ty;
-                match inner_ty.kind() {
-                    ty::Ref(..) => Some(place_base.projection),
-                    _ => None,
+
+                if let ty::Ref(..) = inner_ty.kind() {
+                    return Some(place_base);
+                } else {
+                    return None;
                 }
             }
         }

compiler/rustc_mir/src/transform/simplify.rs

@@ -413,8 +413,7 @@ impl UsedLocals {
         } else {
             // A definition. Although, it still might use other locals for indexing.
             self.super_projection(
-                place.local,
-                &place.projection,
+                place.as_ref(),
                 PlaceContext::MutatingUse(MutatingUseContext::Projection),
                 location,
             );

compiler/rustc_parse/src/parser/mod.rs

@@ -950,7 +950,7 @@ impl<'a> Parser<'a> {
             self.bump();
             Ok(Ident::new(symbol, self.prev_token.span))
         } else {
-            self.parse_ident_common(false)
+            self.parse_ident_common(true)
         }
     }
 

compiler/rustc_parse/src/parser/pat.rs

@@ -1028,7 +1028,7 @@ impl<'a> Parser<'a> {
             let boxed_span = self.token.span;
             let is_ref = self.eat_keyword(kw::Ref);
             let is_mut = self.eat_keyword(kw::Mut);
-            let fieldname = self.parse_ident()?;
+            let fieldname = self.parse_field_name()?;
             hi = self.prev_token.span;
 
             let bind_type = match (is_ref, is_mut) {

compiler/rustc_span/src/symbol.rs

@@ -169,10 +169,14 @@ symbols! {
         Option,
         Ord,
         Ordering,
+        OsStr,
+        OsString,
         Output,
         Param,
         PartialEq,
         PartialOrd,
+        Path,
+        PathBuf,
         Pending,
         Pin,
         Poll,
@@ -198,6 +202,8 @@ symbols! {
         StructuralPartialEq,
         Sync,
         Target,
+        ToOwned,
+        ToString,
         Try,
         Ty,
         TyCtxt,

compiler/rustc_typeck/src/check/callee.rs

@@ -346,7 +346,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                             if call_is_multiline {
                                 err.span_suggestion(
                                     callee.span.shrink_to_hi(),
-                                    "try adding a semicolon",
+                                    "consider using a semicolon here",
                                     ";".to_owned(),
                                     Applicability::MaybeIncorrect,
                                 );

compiler/rustc_typeck/src/check/coercion.rs

@@ -42,6 +42,7 @@ use rustc_hir as hir;
 use rustc_hir::def_id::DefId;
 use rustc_infer::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind};
 use rustc_infer::infer::{Coercion, InferOk, InferResult};
+use rustc_middle::lint::in_external_macro;
 use rustc_middle::ty::adjustment::{
     Adjust, Adjustment, AllowTwoPhase, AutoBorrow, AutoBorrowMutability, PointerCast,
 };
@@ -1448,17 +1449,24 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
                 expected.is_unit(),
                 pointing_at_return_type,
             ) {
-                if cond_expr.span.desugaring_kind().is_none() {
+                // If the block is from an external macro, then do not suggest
+                // adding a semicolon, because there's nowhere to put it.
+                // See issue #81943.
+                if cond_expr.span.desugaring_kind().is_none()
+                    && !in_external_macro(fcx.tcx.sess, cond_expr.span)
+                {
                     err.span_label(cond_expr.span, "expected this to be `()`");
-                    fcx.suggest_semicolon_at_end(cond_expr.span, &mut err);
+                    if expr.can_have_side_effects() {
+                        fcx.suggest_semicolon_at_end(cond_expr.span, &mut err);
+                    }
                 }
             }
             fcx.get_node_fn_decl(parent).map(|(fn_decl, _, is_main)| (fn_decl, is_main))
         } else {
             fcx.get_fn_decl(parent_id)
         };
 
-        if let (Some((fn_decl, can_suggest)), _) = (fn_decl, pointing_at_return_type) {
+        if let Some((fn_decl, can_suggest)) = fn_decl {
             if expression.is_none() {
                 pointing_at_return_type |= fcx.suggest_missing_return_type(
                     &mut err,
@@ -1472,6 +1480,16 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
                 fn_output = Some(&fn_decl.output); // `impl Trait` return type
             }
         }
+
+        let parent_id = fcx.tcx.hir().get_parent_item(id);
+        let parent_item = fcx.tcx.hir().get(parent_id);
+
+        if let (Some((expr, _)), Some((fn_decl, _, _))) =
+            (expression, fcx.get_node_fn_decl(parent_item))
+        {
+            fcx.suggest_missing_return_expr(&mut err, expr, fn_decl, expected, found);
+        }
+
         if let (Some(sp), Some(fn_output)) = (fcx.ret_coercion_span.get(), fn_output) {
             self.add_impl_trait_explanation(&mut err, cause, fcx, expected, sp, fn_output);
         }

compiler/rustc_typeck/src/check/fn_ctxt/checks.rs

@@ -561,7 +561,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             hir::StmtKind::Expr(ref expr) => {
                 // Check with expected type of `()`.
                 self.check_expr_has_type_or_error(&expr, self.tcx.mk_unit(), |err| {
-                    self.suggest_semicolon_at_end(expr.span, err);
+                    if expr.can_have_side_effects() {
+                        self.suggest_semicolon_at_end(expr.span, err);
+                    }
                 });
             }
             hir::StmtKind::Semi(ref expr) => {

compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs

@@ -44,11 +44,14 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         blk_id: hir::HirId,
     ) -> bool {
         let expr = expr.peel_drop_temps();
-        self.suggest_missing_semicolon(err, expr, expected, cause_span);
+        if expr.can_have_side_effects() {
+            self.suggest_missing_semicolon(err, expr, expected, cause_span);
+        }
         let mut pointing_at_return_type = false;
         if let Some((fn_decl, can_suggest)) = self.get_fn_decl(blk_id) {
             pointing_at_return_type =
                 self.suggest_missing_return_type(err, &fn_decl, expected, found, can_suggest);
+            self.suggest_missing_return_expr(err, expr, &fn_decl, expected, found);
         }
         pointing_at_return_type
     }
@@ -392,10 +395,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                 | ExprKind::Loop(..)
                 | ExprKind::If(..)
                 | ExprKind::Match(..)
-                | ExprKind::Block(..) => {
+                | ExprKind::Block(..)
+                    if expression.can_have_side_effects() =>
+                {
                     err.span_suggestion(
                         cause_span.shrink_to_hi(),
-                        "try adding a semicolon",
+                        "consider using a semicolon here",
                         ";".to_string(),
                         Applicability::MachineApplicable,
                     );
@@ -464,6 +469,34 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         }
     }
 
+    pub(in super::super) fn suggest_missing_return_expr(
+        &self,
+        err: &mut DiagnosticBuilder<'_>,
+        expr: &'tcx hir::Expr<'tcx>,
+        fn_decl: &hir::FnDecl<'_>,
+        expected: Ty<'tcx>,
+        found: Ty<'tcx>,
+    ) {
+        if !expected.is_unit() {
+            return;
+        }
+        let found = self.resolve_vars_with_obligations(found);
+        if let hir::FnRetTy::Return(ty) = fn_decl.output {
+            let ty = AstConv::ast_ty_to_ty(self, ty);
+            let ty = self.normalize_associated_types_in(expr.span, ty);
+            if self.can_coerce(found, ty) {
+                err.multipart_suggestion(
+                    "you might have meant to return this value",
+                    vec![
+                        (expr.span.shrink_to_lo(), "return ".to_string()),
+                        (expr.span.shrink_to_hi(), ";".to_string()),
+                    ],
+                    Applicability::MaybeIncorrect,
+                );
+            }
+        }
+    }
+
     pub(in super::super) fn suggest_missing_parentheses(
         &self,
         err: &mut DiagnosticBuilder<'_>,