Auto merge of #103431 - Dylan-DPC:rollup-oozfo89, r=Dylan-DPC

Rollup of 6 pull requests

Successful merges:

 - #101293 (Recover when unclosed char literal is parsed as a lifetime in some positions)
 - #101908 (Suggest let for assignment, and some code refactor)
 - #103192 (rustdoc: Eliminate uses of `EarlyDocLinkResolver::all_traits`)
 - #103226 (Check `needs_infer` before `needs_drop` during HIR generator analysis)
 - #103249 (resolve: Revert "Set effective visibilities for imports more precisely")
 - #103305 (Move some tests to more reasonable places)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup

compiler/rustc_errors/src/lib.rs

@@ -463,6 +463,9 @@ pub enum StashKey {
     UnderscoreForArrayLengths,
     EarlySyntaxWarning,
     CallIntoMethod,
+    /// When an invalid lifetime e.g. `'2` should be reinterpreted
+    /// as a char literal in the parser
+    LifetimeIsChar,
 }
 
 fn default_track_diagnostic(_: &Diagnostic) {}

compiler/rustc_hir_typeck/src/generator_interior/drop_ranges/record_consumed_borrow.rs

@@ -6,8 +6,11 @@ use crate::{
 use hir::{def_id::DefId, Body, HirId, HirIdMap};
 use rustc_data_structures::fx::FxHashSet;
 use rustc_hir as hir;
-use rustc_middle::hir::place::{PlaceBase, Projection, ProjectionKind};
 use rustc_middle::ty::{ParamEnv, TyCtxt};
+use rustc_middle::{
+    hir::place::{PlaceBase, Projection, ProjectionKind},
+    ty::TypeVisitable,
+};
 
 pub(super) fn find_consumed_and_borrowed<'a, 'tcx>(
     fcx: &'a FnCtxt<'a, 'tcx>,
@@ -198,11 +201,13 @@ impl<'tcx> expr_use_visitor::Delegate<'tcx> for ExprUseDelegate<'tcx> {
 
         // If the type being assigned needs dropped, then the mutation counts as a borrow
         // since it is essentially doing `Drop::drop(&mut x); x = new_value;`.
-        //
-        // FIXME(drop-tracking): We need to be more responsible about inference
-        // variables here, since `needs_drop` is a "raw" type query, i.e. it
-        // basically requires types to have been fully resolved.
-        if assignee_place.place.base_ty.needs_drop(self.tcx, self.param_env) {
+        let ty = self.tcx.erase_regions(assignee_place.place.base_ty);
+        if ty.needs_infer() {
+            self.tcx.sess.delay_span_bug(
+                self.tcx.hir().span(assignee_place.hir_id),
+                &format!("inference variables in {ty}"),
+            );
+        } else if ty.needs_drop(self.tcx, self.param_env) {
             self.places
                 .borrowed
                 .insert(TrackedValue::from_place_with_projections_allowed(assignee_place));

compiler/rustc_hir_typeck/src/generator_interior/mod.rs

@@ -377,15 +377,6 @@ impl<'a, 'tcx> Visitor<'tcx> for InteriorVisitor<'a, 'tcx> {
         debug!("is_borrowed_temporary: {:?}", self.drop_ranges.is_borrowed_temporary(expr));
 
         let ty = self.fcx.typeck_results.borrow().expr_ty_adjusted_opt(expr);
-        let may_need_drop = |ty: Ty<'tcx>| {
-            // Avoid ICEs in needs_drop.
-            let ty = self.fcx.resolve_vars_if_possible(ty);
-            let ty = self.fcx.tcx.erase_regions(ty);
-            if ty.needs_infer() {
-                return true;
-            }
-            ty.needs_drop(self.fcx.tcx, self.fcx.param_env)
-        };
 
         // Typically, the value produced by an expression is consumed by its parent in some way,
         // so we only have to check if the parent contains a yield (note that the parent may, for
@@ -403,9 +394,18 @@ impl<'a, 'tcx> Visitor<'tcx> for InteriorVisitor<'a, 'tcx> {
         // src/test/ui/generator/drop-tracking-parent-expression.rs.
         let scope = if self.drop_ranges.is_borrowed_temporary(expr)
             || ty.map_or(true, |ty| {
-                let needs_drop = may_need_drop(ty);
-                debug!(?needs_drop, ?ty);
-                needs_drop
+                // Avoid ICEs in needs_drop.
+                let ty = self.fcx.resolve_vars_if_possible(ty);
+                let ty = self.fcx.tcx.erase_regions(ty);
+                if ty.needs_infer() {
+                    self.fcx
+                        .tcx
+                        .sess
+                        .delay_span_bug(expr.span, &format!("inference variables in {ty}"));
+                    true
+                } else {
+                    ty.needs_drop(self.fcx.tcx, self.fcx.param_env)
+                }
             }) {
             self.rvalue_scopes.temporary_scope(self.region_scope_tree, expr.hir_id.local_id)
         } else {

compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs

@@ -587,11 +587,6 @@ impl CStore {
         self.get_crate_data(cnum).get_proc_macro_quoted_span(id, sess)
     }
 
-    /// Decodes all traits in the crate (for rustdoc).
-    pub fn traits_in_crate_untracked(&self, cnum: CrateNum) -> impl Iterator<Item = DefId> + '_ {
-        self.get_crate_data(cnum).get_traits()
-    }
-
     /// Decodes all trait impls in the crate (for rustdoc).
     pub fn trait_impls_in_crate_untracked(
         &self,

compiler/rustc_parse/src/lexer/mod.rs

@@ -3,7 +3,9 @@ use rustc_ast::ast::{self, AttrStyle};
 use rustc_ast::token::{self, CommentKind, Delimiter, Token, TokenKind};
 use rustc_ast::tokenstream::TokenStream;
 use rustc_ast::util::unicode::contains_text_flow_control_chars;
-use rustc_errors::{error_code, Applicability, DiagnosticBuilder, ErrorGuaranteed, PResult};
+use rustc_errors::{
+    error_code, Applicability, DiagnosticBuilder, ErrorGuaranteed, PResult, StashKey,
+};
 use rustc_lexer::unescape::{self, Mode};
 use rustc_lexer::Cursor;
 use rustc_lexer::{Base, DocStyle, RawStrError};
@@ -203,7 +205,10 @@ impl<'a> StringReader<'a> {
                     // this is necessary.
                     let lifetime_name = self.str_from(start);
                     if starts_with_number {
-                        self.err_span_(start, self.pos, "lifetimes cannot start with a number");
+                        let span = self.mk_sp(start, self.pos);
+                        let mut diag = self.sess.struct_err("lifetimes cannot start with a number");
+                        diag.set_span(span);
+                        diag.stash(span, StashKey::LifetimeIsChar);
                     }
                     let ident = Symbol::intern(lifetime_name);
                     token::Lifetime(ident)

compiler/rustc_parse/src/parser/expr.rs

@@ -42,8 +42,10 @@ use rustc_ast::{AnonConst, BinOp, BinOpKind, FnDecl, FnRetTy, MacCall, Param, Ty
 use rustc_ast::{Arm, Async, BlockCheckMode, Expr, ExprKind, Label, Movability, RangeLimits};
 use rustc_ast::{ClosureBinder, StmtKind};
 use rustc_ast_pretty::pprust;
-use rustc_errors::IntoDiagnostic;
-use rustc_errors::{Applicability, Diagnostic, PResult};
+use rustc_errors::{
+    Applicability, Diagnostic, DiagnosticBuilder, ErrorGuaranteed, IntoDiagnostic, PResult,
+    StashKey,
+};
 use rustc_session::errors::ExprParenthesesNeeded;
 use rustc_session::lint::builtin::BREAK_WITH_LABEL_AND_LOOP;
 use rustc_session::lint::BuiltinLintDiagnostics;
@@ -1513,11 +1515,11 @@ impl<'a> Parser<'a> {
     /// Parse `'label: $expr`. The label is already parsed.
     fn parse_labeled_expr(
         &mut self,
-        label: Label,
+        label_: Label,
         mut consume_colon: bool,
     ) -> PResult<'a, P<Expr>> {
-        let lo = label.ident.span;
-        let label = Some(label);
+        let lo = label_.ident.span;
+        let label = Some(label_);
         let ate_colon = self.eat(&token::Colon);
         let expr = if self.eat_keyword(kw::While) {
             self.parse_while_expr(label, lo)
@@ -1529,6 +1531,19 @@ impl<'a> Parser<'a> {
             || self.token.is_whole_block()
         {
             self.parse_block_expr(label, lo, BlockCheckMode::Default)
+        } else if !ate_colon
+            && (matches!(self.token.kind, token::CloseDelim(_) | token::Comma)
+                || self.token.is_op())
+        {
+            let lit = self.recover_unclosed_char(label_.ident, |self_| {
+                self_.sess.create_err(UnexpectedTokenAfterLabel {
+                    span: self_.token.span,
+                    remove_label: None,
+                    enclose_in_block: None,
+                })
+            });
+            consume_colon = false;
+            Ok(self.mk_expr(lo, ExprKind::Lit(lit)))
         } else if !ate_colon
             && (self.check_noexpect(&TokenKind::Comma) || self.check_noexpect(&TokenKind::Gt))
         {
@@ -1603,6 +1618,39 @@ impl<'a> Parser<'a> {
         Ok(expr)
     }
 
+    /// Emit an error when a char is parsed as a lifetime because of a missing quote
+    pub(super) fn recover_unclosed_char(
+        &mut self,
+        lifetime: Ident,
+        err: impl FnOnce(&mut Self) -> DiagnosticBuilder<'a, ErrorGuaranteed>,
+    ) -> ast::Lit {
+        if let Some(mut diag) =
+            self.sess.span_diagnostic.steal_diagnostic(lifetime.span, StashKey::LifetimeIsChar)
+        {
+            diag.span_suggestion_verbose(
+                lifetime.span.shrink_to_hi(),
+                "add `'` to close the char literal",
+                "'",
+                Applicability::MaybeIncorrect,
+            )
+            .emit();
+        } else {
+            err(self)
+                .span_suggestion_verbose(
+                    lifetime.span.shrink_to_hi(),
+                    "add `'` to close the char literal",
+                    "'",
+                    Applicability::MaybeIncorrect,
+                )
+                .emit();
+        }
+        ast::Lit {
+            token_lit: token::Lit::new(token::LitKind::Char, lifetime.name, None),
+            kind: ast::LitKind::Char(lifetime.name.as_str().chars().next().unwrap_or('_')),
+            span: lifetime.span,
+        }
+    }
+
     /// Recover on the syntax `do catch { ... }` suggesting `try { ... }` instead.
     fn recover_do_catch(&mut self) -> PResult<'a, P<Expr>> {
         let lo = self.token.span;
@@ -1728,7 +1776,7 @@ impl<'a> Parser<'a> {
     }
 
     pub(super) fn parse_lit(&mut self) -> PResult<'a, Lit> {
-        self.parse_opt_lit().ok_or_else(|| {
+        self.parse_opt_lit().ok_or(()).or_else(|()| {
             if let token::Interpolated(inner) = &self.token.kind {
                 let expr = match inner.as_ref() {
                     token::NtExpr(expr) => Some(expr),
@@ -1740,12 +1788,22 @@ impl<'a> Parser<'a> {
                         let mut err = InvalidInterpolatedExpression { span: self.token.span }
                             .into_diagnostic(&self.sess.span_diagnostic);
                         err.downgrade_to_delayed_bug();
-                        return err;
+                        return Err(err);
                     }
                 }
             }
-            let msg = format!("unexpected token: {}", super::token_descr(&self.token));
-            self.struct_span_err(self.token.span, &msg)
+            let token = self.token.clone();
+            let err = |self_: &mut Self| {
+                let msg = format!("unexpected token: {}", super::token_descr(&token));
+                self_.struct_span_err(token.span, &msg)
+            };
+            // On an error path, eagerly consider a lifetime to be an unclosed character lit
+            if self.token.is_lifetime() {
+                let lt = self.expect_lifetime();
+                Ok(self.recover_unclosed_char(lt.ident, err))
+            } else {
+                Err(err(self))
+            }
         })
     }
 

compiler/rustc_parse/src/parser/pat.rs

@@ -402,6 +402,25 @@ impl<'a> Parser<'a> {
             } else {
                 PatKind::Path(qself, path)
             }
+        } else if matches!(self.token.kind, token::Lifetime(_))
+            // In pattern position, we're totally fine with using "next token isn't colon"
+            // as a heuristic. We could probably just always try to recover if it's a lifetime,
+            // because we never have `'a: label {}` in a pattern position anyways, but it does
+            // keep us from suggesting something like `let 'a: Ty = ..` => `let 'a': Ty = ..`
+            && !self.look_ahead(1, |token| matches!(token.kind, token::Colon))
+        {
+            // Recover a `'a` as a `'a'` literal
+            let lt = self.expect_lifetime();
+            let lit = self.recover_unclosed_char(lt.ident, |self_| {
+                let expected = expected.unwrap_or("pattern");
+                let msg =
+                    format!("expected {}, found {}", expected, super::token_descr(&self_.token));
+
+                let mut err = self_.struct_span_err(self_.token.span, &msg);
+                err.span_label(self_.token.span, format!("expected {}", expected));
+                err
+            });
+            PatKind::Lit(self.mk_expr(lo, ExprKind::Lit(lit)))
         } else {
             // Try to parse everything else as literal with optional minus
             match self.parse_literal_maybe_minus() {
@@ -799,6 +818,7 @@ impl<'a> Parser<'a> {
                 || t.kind == token::Dot // e.g. `.5` for recovery;
                 || t.can_begin_literal_maybe_minus() // e.g. `42`.
                 || t.is_whole_expr()
+                || t.is_lifetime() // recover `'a` instead of `'a'`
             })
     }
 

compiler/rustc_resolve/src/access_levels.rs

@@ -1,5 +1,4 @@
-use crate::NameBindingKind;
-use crate::Resolver;
+use crate::{ImportKind, NameBindingKind, Resolver};
 use rustc_ast::ast;
 use rustc_ast::visit;
 use rustc_ast::visit::Visitor;
@@ -45,31 +44,40 @@ impl<'r, 'a> AccessLevelsVisitor<'r, 'a> {
         let module = self.r.get_module(module_id.to_def_id()).unwrap();
         let resolutions = self.r.resolutions(module);
 
-        for (key, name_resolution) in resolutions.borrow().iter() {
+        for (_, name_resolution) in resolutions.borrow().iter() {
             if let Some(mut binding) = name_resolution.borrow().binding() && !binding.is_ambiguity() {
                 // Set the given binding access level to `AccessLevel::Public` and
                 // sets the rest of the `use` chain to `AccessLevel::Exported` until
                 // we hit the actual exported item.
 
-                // FIXME: tag and is_public() condition must be deleted,
-                // but assertion fail occurs in import_id_for_ns
+                // FIXME: tag and is_public() condition should be removed, but assertions occur.
                 let tag = if binding.is_import() { AccessLevel::Exported } else { AccessLevel::Public };
                 if binding.vis.is_public() {
                     let mut prev_parent_id = module_id;
                     let mut level = AccessLevel::Public;
                     while let NameBindingKind::Import { binding: nested_binding, import, .. } =
                         binding.kind
                     {
-                        let id = self.r.local_def_id(self.r.import_id_for_ns(import, key.ns));
-                        self.update(
-                            id,
+                        let mut update = |node_id| self.update(
+                            self.r.local_def_id(node_id),
                             binding.vis.expect_local(),
                             prev_parent_id,
                             level,
                         );
+                        // In theory all the import IDs have individual visibilities and effective
+                        // visibilities, but in practice these IDs go straigth to HIR where all
+                        // their few uses assume that their (effective) visibility applies to the
+                        // whole syntactic `use` item. So we update them all to the maximum value
+                        // among the potential individual effective visibilities. Maybe HIR for
+                        // imports shouldn't use three IDs at all.
+                        update(import.id);
+                        if let ImportKind::Single { additional_ids, .. } = import.kind {
+                            update(additional_ids.0);
+                            update(additional_ids.1);
+                        }
 
                         level = AccessLevel::Exported;
-                        prev_parent_id = id;
+                        prev_parent_id = self.r.local_def_id(import.id);
                         binding = nested_binding;
                     }
                 }

compiler/rustc_resolve/src/imports.rs

@@ -2,7 +2,7 @@
 
 use crate::diagnostics::{import_candidates, Suggestion};
 use crate::Determinacy::{self, *};
-use crate::Namespace::{self, *};
+use crate::Namespace::*;
 use crate::{module_to_string, names_to_string, ImportSuggestion};
 use crate::{AmbiguityKind, BindingKey, ModuleKind, ResolutionError, Resolver, Segment};
 use crate::{Finalize, Module, ModuleOrUniformRoot, ParentScope, PerNS, ScopeSet};
@@ -371,31 +371,6 @@ impl<'a> Resolver<'a> {
             self.used_imports.insert(import.id);
         }
     }
-
-    /// Take primary and additional node IDs from an import and select one that corresponds to the
-    /// given namespace. The logic must match the corresponding logic from `fn lower_use_tree` that
-    /// assigns resolutons to IDs.
-    pub(crate) fn import_id_for_ns(&self, import: &Import<'_>, ns: Namespace) -> NodeId {
-        if let ImportKind::Single { additional_ids: (id1, id2), .. } = import.kind {
-            if let Some(resolutions) = self.import_res_map.get(&import.id) {
-                assert!(resolutions[ns].is_some(), "incorrectly finalized import");
-                return match ns {
-                    TypeNS => import.id,
-                    ValueNS => match resolutions.type_ns {
-                        Some(_) => id1,
-                        None => import.id,
-                    },
-                    MacroNS => match (resolutions.type_ns, resolutions.value_ns) {
-                        (Some(_), Some(_)) => id2,
-                        (Some(_), None) | (None, Some(_)) => id1,
-                        (None, None) => import.id,
-                    },
-                };
-            }
-        }
-
-        import.id
-    }
 }
 
 /// An error that may be transformed into a diagnostic later. Used to combine multiple unresolved

compiler/rustc_resolve/src/late.rs

@@ -524,6 +524,9 @@ struct DiagnosticMetadata<'ast> {
     /// Used to detect possible `if let` written without `let` and to provide structured suggestion.
     in_if_condition: Option<&'ast Expr>,
 
+    /// Used to detect possible new binding written without `let` and to provide structured suggestion.
+    in_assignment: Option<&'ast Expr>,
+
     /// If we are currently in a trait object definition. Used to point at the bounds when
     /// encountering a struct or enum.
     current_trait_object: Option<&'ast [ast::GenericBound]>,
@@ -3905,6 +3908,11 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
                 self.resolve_expr(elem, Some(expr));
                 self.visit_expr(idx);
             }
+            ExprKind::Assign(..) => {
+                let old = self.diagnostic_metadata.in_assignment.replace(expr);
+                visit::walk_expr(self, expr);
+                self.diagnostic_metadata.in_assignment = old;
+            }
             _ => {
                 visit::walk_expr(self, expr);
             }