Auto merge of #120163 - GuillaumeGomez:rollup-denmbz8, r=GuillaumeGomez

Rollup of 6 pull requests Successful merges: - #118257 (Make traits / trait methods detected by the dead code lint) - #119997 (Fix impl stripped in rustdoc HTML whereas it should not be in case the impl is implemented on a type alias) - #120000 (Ensure `callee_id`s are body owners) - #120015 (coverage: Format all coverage tests with `rustfmt`) - #120063 (Remove special handling of `box` expressions from parser) - #120138 (Increase vscode settings.json `git.detectSubmodulesLimit`) r? `@ghost` `@rustbot` modify labels: rollup
rust-lang · Jan 20, 2024 · e13d172 · e13d172
2 parents 159bdc1 + da2811e
commit e13d172
Show file tree

Hide file tree

Showing 386 changed files with 1,530 additions and 531 deletions.
diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs
@@ -33,7 +33,6 @@ use rustc_session::errors::{report_lit_error, ExprParenthesesNeeded};
 use rustc_session::lint::builtin::BREAK_WITH_LABEL_AND_LOOP;
 use rustc_session::lint::BuiltinLintDiagnostics;
 use rustc_span::source_map::{self, Spanned};
-use rustc_span::symbol::kw::PathRoot;
 use rustc_span::symbol::{kw, sym, Ident, Symbol};
 use rustc_span::{BytePos, Pos, Span};
 use thin_vec::{thin_vec, ThinVec};
@@ -642,26 +641,13 @@ impl<'a> Parser<'a> {
     }
 
     /// Parse `box expr` - this syntax has been removed, but we still parse this
-    /// for now to provide an automated way to fix usages of it
-    fn parse_expr_box(&mut self, lo: Span) -> PResult<'a, (Span, ExprKind)> {
-        let (span, expr) = self.parse_expr_prefix_common(lo)?;
-        let code = self.sess.source_map().span_to_snippet(span.with_lo(lo.hi())).unwrap();
-        self.dcx().emit_err(errors::BoxSyntaxRemoved { span, code: code.trim() });
-        // So typechecking works, parse `box <expr>` as `::std::boxed::Box::new(expr)`
-        let path = Path {
-            span,
-            segments: [
-                PathSegment::from_ident(Ident::with_dummy_span(PathRoot)),
-                PathSegment::from_ident(Ident::with_dummy_span(sym::std)),
-                PathSegment::from_ident(Ident::from_str("boxed")),
-                PathSegment::from_ident(Ident::from_str("Box")),
-                PathSegment::from_ident(Ident::with_dummy_span(sym::new)),
-            ]
-            .into(),
-            tokens: None,
-        };
-        let path = self.mk_expr(span, ExprKind::Path(None, path));
-        Ok((span, self.mk_call(path, ThinVec::from([expr]))))
+    /// for now to provide a more useful error
+    fn parse_expr_box(&mut self, box_kw: Span) -> PResult<'a, (Span, ExprKind)> {
+        let (span, _) = self.parse_expr_prefix_common(box_kw)?;
+        let inner_span = span.with_lo(box_kw.hi());
+        let code = self.sess.source_map().span_to_snippet(inner_span).unwrap();
+        self.dcx().emit_err(errors::BoxSyntaxRemoved { span: span, code: code.trim() });
+        Ok((span, ExprKind::Err))
     }
 
     fn is_mistaken_not_ident_negation(&self) -> bool {

diff --git a/compiler/rustc_passes/src/dead.rs b/compiler/rustc_passes/src/dead.rs
@@ -4,6 +4,7 @@
 // is dead.
 
 use hir::def_id::{LocalDefIdMap, LocalDefIdSet};
+use hir::ItemKind;
 use rustc_data_structures::unord::UnordSet;
 use rustc_errors::MultiSpan;
 use rustc_hir as hir;
@@ -14,7 +15,7 @@ use rustc_hir::{Node, PatKind, TyKind};
 use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags;
 use rustc_middle::middle::privacy::Level;
 use rustc_middle::query::Providers;
-use rustc_middle::ty::{self, TyCtxt};
+use rustc_middle::ty::{self, TyCtxt, Visibility};
 use rustc_session::lint;
 use rustc_session::lint::builtin::DEAD_CODE;
 use rustc_span::symbol::{sym, Symbol};
@@ -381,9 +382,46 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
                     intravisit::walk_item(self, item)
                 }
                 hir::ItemKind::ForeignMod { .. } => {}
+                hir::ItemKind::Trait(..) => {
+                    for impl_def_id in self.tcx.all_impls(item.owner_id.to_def_id()) {
+                        if let Some(local_def_id) = impl_def_id.as_local()
+                            && let ItemKind::Impl(impl_ref) =
+                                self.tcx.hir().expect_item(local_def_id).kind
+                        {
+                            // skip items
+                            // mark dependent traits live
+                            intravisit::walk_generics(self, impl_ref.generics);
+                            // mark dependent parameters live
+                            intravisit::walk_path(self, impl_ref.of_trait.unwrap().path);
+                        }
+                    }
+
+                    intravisit::walk_item(self, item)
+                }
                 _ => intravisit::walk_item(self, item),
             },
             Node::TraitItem(trait_item) => {
+                // mark corresponing ImplTerm live
+                let trait_item_id = trait_item.owner_id.to_def_id();
+                if let Some(trait_id) = self.tcx.trait_of_item(trait_item_id) {
+                    // mark the trait live
+                    self.check_def_id(trait_id);
+
+                    for impl_id in self.tcx.all_impls(trait_id) {
+                        if let Some(local_impl_id) = impl_id.as_local()
+                            && let ItemKind::Impl(impl_ref) =
+                                self.tcx.hir().expect_item(local_impl_id).kind
+                        {
+                            // mark self_ty live
+                            intravisit::walk_ty(self, impl_ref.self_ty);
+                            if let Some(&impl_item_id) =
+                                self.tcx.impl_item_implementor_ids(impl_id).get(&trait_item_id)
+                            {
+                                self.check_def_id(impl_item_id);
+                            }
+                        }
+                    }
+                }
                 intravisit::walk_trait_item(self, trait_item);
             }
             Node::ImplItem(impl_item) => {
@@ -632,10 +670,6 @@ fn check_item<'tcx>(
             }
         }
         DefKind::Impl { of_trait } => {
-            if of_trait {
-                worklist.push((id.owner_id.def_id, ComesFromAllowExpect::No));
-            }
-
             // get DefIds from another query
             let local_def_ids = tcx
                 .associated_item_def_ids(id.owner_id)
@@ -644,7 +678,11 @@ fn check_item<'tcx>(
 
             // And we access the Map here to get HirId from LocalDefId
             for id in local_def_ids {
-                if of_trait {
+                // for impl trait blocks, mark associate functions live if the trait is public
+                if of_trait
+                    && (!matches!(tcx.def_kind(id), DefKind::AssocFn)
+                        || tcx.local_visibility(id) == Visibility::Public)
+                {
                     worklist.push((id, ComesFromAllowExpect::No));
                 } else if let Some(comes_from_allow) = has_allow_dead_code_or_lang_attr(tcx, id) {
                     worklist.push((id, comes_from_allow));
@@ -675,7 +713,7 @@ fn check_trait_item(
     use hir::TraitItemKind::{Const, Fn};
     if matches!(tcx.def_kind(id.owner_id), DefKind::AssocConst | DefKind::AssocFn) {
         let trait_item = tcx.hir().trait_item(id);
-        if matches!(trait_item.kind, Const(_, Some(_)) | Fn(_, hir::TraitFn::Provided(_)))
+        if matches!(trait_item.kind, Const(_, Some(_)) | Fn(..))
             && let Some(comes_from_allow) =
                 has_allow_dead_code_or_lang_attr(tcx, trait_item.owner_id.def_id)
         {
@@ -944,7 +982,8 @@ impl<'tcx> DeadVisitor<'tcx> {
             | DefKind::TyAlias
             | DefKind::Enum
             | DefKind::Union
-            | DefKind::ForeignTy => self.warn_dead_code(def_id, "used"),
+            | DefKind::ForeignTy
+            | DefKind::Trait => self.warn_dead_code(def_id, "used"),
             DefKind::Struct => self.warn_dead_code(def_id, "constructed"),
             DefKind::Variant | DefKind::Field => bug!("should be handled specially"),
             _ => {}
@@ -969,18 +1008,33 @@ fn check_mod_deathness(tcx: TyCtxt<'_>, module: LocalModDefId) {
     let module_items = tcx.hir_module_items(module);
 
     for item in module_items.items() {
-        if let hir::ItemKind::Impl(impl_item) = tcx.hir().item(item).kind {
-            let mut dead_items = Vec::new();
-            for item in impl_item.items {
-                let def_id = item.id.owner_id.def_id;
-                if !visitor.is_live_code(def_id) {
-                    let name = tcx.item_name(def_id.to_def_id());
-                    let level = visitor.def_lint_level(def_id);
+        let def_kind = tcx.def_kind(item.owner_id);
 
-                    dead_items.push(DeadItem { def_id, name, level })
+        let mut dead_codes = Vec::new();
+        // if we have diagnosed the trait, do not diagnose unused methods
+        if matches!(def_kind, DefKind::Impl { .. })
+            || (def_kind == DefKind::Trait && live_symbols.contains(&item.owner_id.def_id))
+        {
+            for &def_id in tcx.associated_item_def_ids(item.owner_id.def_id) {
+                // We have diagnosed unused methods in traits
+                if matches!(def_kind, DefKind::Impl { of_trait: true })
+                    && tcx.def_kind(def_id) == DefKind::AssocFn
+                    || def_kind == DefKind::Trait && tcx.def_kind(def_id) != DefKind::AssocFn
+                {
+                    continue;
+                }
+
+                if let Some(local_def_id) = def_id.as_local()
+                    && !visitor.is_live_code(local_def_id)
+                {
+                    let name = tcx.item_name(def_id);
+                    let level = visitor.def_lint_level(local_def_id);
+                    dead_codes.push(DeadItem { def_id: local_def_id, name, level });
                 }
             }
-            visitor.warn_multiple(item.owner_id.def_id, "used", dead_items, ReportOn::NamedField);
+        }
+        if !dead_codes.is_empty() {
+            visitor.warn_multiple(item.owner_id.def_id, "used", dead_codes, ReportOn::NamedField);
         }
 
         if !live_symbols.contains(&item.owner_id.def_id) {
@@ -993,7 +1047,6 @@ fn check_mod_deathness(tcx: TyCtxt<'_>, module: LocalModDefId) {
             continue;
         }
 
-        let def_kind = tcx.def_kind(item.owner_id);
         if let DefKind::Struct | DefKind::Union | DefKind::Enum = def_kind {
             let adt = tcx.adt_def(item.owner_id);
             let mut dead_variants = Vec::new();
@@ -1040,8 +1093,6 @@ fn check_mod_deathness(tcx: TyCtxt<'_>, module: LocalModDefId) {
     for foreign_item in module_items.foreign_items() {
         visitor.check_definition(foreign_item.owner_id.def_id);
     }
-
-    // We do not warn trait items.
 }
 
 pub(crate) fn provide(providers: &mut Providers) {

diff --git a/compiler/rustc_span/src/source_map/tests.rs b/compiler/rustc_span/src/source_map/tests.rs
@@ -1,5 +1,7 @@
 use super::*;
 
+use rustc_data_structures::sync::FreezeLock;
+
 fn init_source_map() -> SourceMap {
     let sm = SourceMap::new(FilePathMapping::empty());
     sm.new_source_file(PathBuf::from("blork.rs").into(), "first line.\nsecond line".to_string());
@@ -263,53 +265,6 @@ fn t10() {
     );
 }
 
-/// Returns the span corresponding to the `n`th occurrence of `substring` in `source_text`.
-trait SourceMapExtension {
-    fn span_substr(
-        &self,
-        file: &Lrc<SourceFile>,
-        source_text: &str,
-        substring: &str,
-        n: usize,
-    ) -> Span;
-}
-
-impl SourceMapExtension for SourceMap {
-    fn span_substr(
-        &self,
-        file: &Lrc<SourceFile>,
-        source_text: &str,
-        substring: &str,
-        n: usize,
-    ) -> Span {
-        eprintln!(
-            "span_substr(file={:?}/{:?}, substring={:?}, n={})",
-            file.name, file.start_pos, substring, n
-        );
-        let mut i = 0;
-        let mut hi = 0;
-        loop {
-            let offset = source_text[hi..].find(substring).unwrap_or_else(|| {
-                panic!(
-                    "source_text `{}` does not have {} occurrences of `{}`, only {}",
-                    source_text, n, substring, i
-                );
-            });
-            let lo = hi + offset;
-            hi = lo + substring.len();
-            if i == n {
-                let span = Span::with_root_ctxt(
-                    BytePos(lo as u32 + file.start_pos.0),
-                    BytePos(hi as u32 + file.start_pos.0),
-                );
-                assert_eq!(&self.span_to_snippet(span).unwrap()[..], substring);
-                return span;
-            }
-            i += 1;
-        }
-    }
-}
-
 // Takes a unix-style path and returns a platform specific path.
 fn path(p: &str) -> PathBuf {
     path_str(p).into()

diff --git a/library/core/tests/macros.rs b/library/core/tests/macros.rs
@@ -1,3 +1,4 @@
+#[allow(dead_code)]
 trait Trait {
     fn blah(&self);
 }

diff --git a/src/bootstrap/src/core/build_steps/setup.rs b/src/bootstrap/src/core/build_steps/setup.rs
@@ -37,6 +37,7 @@ static SETTINGS_HASHES: &[&str] = &[
     "3468fea433c25fff60be6b71e8a215a732a7b1268b6a83bf10d024344e140541",
     "47d227f424bf889b0d899b9cc992d5695e1b78c406e183cd78eafefbe5488923",
     "b526bd58d0262dd4dda2bff5bc5515b705fb668a46235ace3e057f807963a11a",
+    "828666b021d837a33e78d870b56d34c88a5e2c85de58b693607ec574f0c27000",
 ];
 static RUST_ANALYZER_SETTINGS: &str = include_str!("../../../../etc/rust_analyzer_settings.json");
 

diff --git a/src/etc/rust_analyzer_settings.json b/src/etc/rust_analyzer_settings.json
@@ -1,4 +1,5 @@
 {
+    "git.detectSubmodulesLimit": 20,
     "rust-analyzer.check.invocationLocation": "root",
     "rust-analyzer.check.invocationStrategy": "once",
     "rust-analyzer.check.overrideCommand": [

diff --git a/src/librustdoc/passes/stripper.rs b/src/librustdoc/passes/stripper.rs
@@ -56,13 +56,10 @@ impl<'a, 'tcx> DocFolder for Stripper<'a, 'tcx> {
             | clean::TraitItem(..)
             | clean::FunctionItem(..)
             | clean::VariantItem(..)
-            | clean::MethodItem(..)
             | clean::ForeignFunctionItem(..)
             | clean::ForeignStaticItem(..)
             | clean::ConstantItem(..)
             | clean::UnionItem(..)
-            | clean::AssocConstItem(..)
-            | clean::AssocTypeItem(..)
             | clean::TraitAliasItem(..)
             | clean::MacroItem(..)
             | clean::ForeignTypeItem => {
@@ -80,6 +77,16 @@ impl<'a, 'tcx> DocFolder for Stripper<'a, 'tcx> {
                 }
             }
 
+            clean::MethodItem(..) | clean::AssocConstItem(..) | clean::AssocTypeItem(..) => {
+                let item_id = i.item_id;
+                if item_id.is_local()
+                    && !self.effective_visibilities.is_reachable(self.tcx, item_id.expect_def_id())
+                {
+                    debug!("Stripper: stripping {:?} {:?}", i.type_(), i.name);
+                    return None;
+                }
+            }
+
             clean::StructFieldItem(..) => {
                 if i.visibility(self.tcx) != Some(Visibility::Public) {
                     return Some(strip_item(i));
@@ -192,16 +199,16 @@ impl<'a> DocFolder for ImplStripper<'a, '_> {
                     && imp.items.iter().all(|i| {
                         let item_id = i.item_id;
                         item_id.is_local()
-                            && !is_item_reachable(
-                                self.tcx,
-                                self.is_json_output,
-                                &self.cache.effective_visibilities,
-                                item_id,
-                            )
+                            && !self
+                                .cache
+                                .effective_visibilities
+                                .is_reachable(self.tcx, item_id.expect_def_id())
                     })
                 {
+                    debug!("ImplStripper: no public item; removing {imp:?}");
                     return None;
                 } else if imp.items.is_empty() && i.doc_value().is_empty() {
+                    debug!("ImplStripper: no item and no doc; removing {imp:?}");
                     return None;
                 }
             }
@@ -212,21 +219,21 @@ impl<'a> DocFolder for ImplStripper<'a, '_> {
                 && !imp.for_.is_assoc_ty()
                 && !self.should_keep_impl(&i, did)
             {
-                debug!("ImplStripper: impl item for stripped type; removing");
+                debug!("ImplStripper: impl item for stripped type; removing {imp:?}");
                 return None;
             }
             if let Some(did) = imp.trait_.as_ref().map(|t| t.def_id())
                 && !self.should_keep_impl(&i, did)
             {
-                debug!("ImplStripper: impl item for stripped trait; removing");
+                debug!("ImplStripper: impl item for stripped trait; removing {imp:?}");
                 return None;
             }
             if let Some(generics) = imp.trait_.as_ref().and_then(|t| t.generics()) {
                 for typaram in generics {
                     if let Some(did) = typaram.def_id(self.cache)
                         && !self.should_keep_impl(&i, did)
                     {
-                        debug!("ImplStripper: stripped item in trait's generics; removing impl");
+                        debug!("ImplStripper: stripped item in trait's generics; removing {imp:?}");
                         return None;
                     }
                 }

diff --git a/src/tools/clippy/clippy_lints/src/derive.rs b/src/tools/clippy/clippy_lints/src/derive.rs
@@ -451,12 +451,12 @@ fn check_partial_eq_without_eq<'tcx>(cx: &LateContext<'tcx>, span: Span, trait_r
         && let Some(def_id) = trait_ref.trait_def_id()
         && cx.tcx.is_diagnostic_item(sym::PartialEq, def_id)
         && let param_env = param_env_for_derived_eq(cx.tcx, adt.did(), eq_trait_def_id)
-        && !implements_trait_with_env(cx.tcx, param_env, ty, eq_trait_def_id, adt.did(),&[])
+        && !implements_trait_with_env(cx.tcx, param_env, ty, eq_trait_def_id, None, &[])
         // If all of our fields implement `Eq`, we can implement `Eq` too
         && adt
             .all_fields()
             .map(|f| f.ty(cx.tcx, args))
-            .all(|ty| implements_trait_with_env(cx.tcx, param_env, ty, eq_trait_def_id, adt.did(), &[]))
+            .all(|ty| implements_trait_with_env(cx.tcx, param_env, ty, eq_trait_def_id, None, &[]))
     {
         span_lint_and_sugg(
             cx,

diff --git a/src/tools/clippy/clippy_lints/src/loops/explicit_iter_loop.rs b/src/tools/clippy/clippy_lints/src/loops/explicit_iter_loop.rs
@@ -118,7 +118,7 @@ fn is_ref_iterable<'tcx>(
             .liberate_late_bound_regions(fn_id, cx.tcx.fn_sig(fn_id).skip_binder())
         && let &[req_self_ty, req_res_ty] = &**sig.inputs_and_output
         && let param_env = cx.tcx.param_env(fn_id)
-        && implements_trait_with_env(cx.tcx, param_env, req_self_ty, trait_id, fn_id, &[])
+        && implements_trait_with_env(cx.tcx, param_env, req_self_ty, trait_id, Some(fn_id), &[])
         && let Some(into_iter_ty) =
             make_normalized_projection_with_regions(cx.tcx, param_env, trait_id, sym!(IntoIter), [req_self_ty])
         && let req_res_ty = normalize_with_regions(cx.tcx, param_env, req_res_ty)