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

Rollup of 6 pull requests #120163

Closed
wants to merge 18 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
999a162
Make traits / trait methods detected by the dead code lint!
mu001999 Jan 1, 2024
155ca3d
Update tests
mu001999 Jan 15, 2024
65409c4
Ensure `callee_id`s are body owners
smoelius Jan 15, 2024
f149442
coverage: Add `#[rustfmt::skip]` to tests with non-standard formatting
Zalathar Jan 16, 2024
1f9353a
coverage: Tweak individual tests to be unaffected by `rustfmt`
Zalathar Jan 16, 2024
b9ebeee
Remove `box <expr>` recovery
clubby789 Jan 17, 2024
3f7c784
Move `removed-syntax` tests to their own directory
clubby789 Jan 17, 2024
99797bb
coverage: Format all remaining tests
Zalathar Jan 16, 2024
96b8eb7
Apply suggestions from code review
smoelius Jan 18, 2024
5afe139
Increase vscode `git.detectSubmodulesLimit`
trevyn Jan 19, 2024
6237beb
Fix impl stripped in rustdoc HTML whereas it should not be in case th…
GuillaumeGomez Jan 15, 2024
0933f48
Add regression test for #119015 and update tests
GuillaumeGomez Jan 15, 2024
c326059
Rollup merge of #118257 - mu001999:dead_code/trait, r=cjgillot
GuillaumeGomez Jan 20, 2024
ba07767
Rollup merge of #119997 - GuillaumeGomez:fix-stripped-impl-on-ty-alia…
GuillaumeGomez Jan 20, 2024
3ee4404
Rollup merge of #120000 - smoelius:fix-clippy, r=fee1-dead
GuillaumeGomez Jan 20, 2024
8a52196
Rollup merge of #120015 - Zalathar:format, r=dtolnay
GuillaumeGomez Jan 20, 2024
8625f36
Rollup merge of #120063 - clubby789:remove-box-handling, r=Nilstrieb
GuillaumeGomez Jan 20, 2024
da2811e
Rollup merge of #120138 - trevyn:detect-submodules-limit, r=albertlar…
GuillaumeGomez Jan 20, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
28 changes: 7 additions & 21 deletions compiler/rustc_parse/src/parser/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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 {
Expand Down
91 changes: 71 additions & 20 deletions compiler/rustc_passes/src/dead.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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};
Expand Down Expand Up @@ -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) => {
Expand Down Expand Up @@ -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)
Expand All @@ -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));
Expand Down Expand Up @@ -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)
{
Expand Down Expand Up @@ -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"),
_ => {}
Expand All @@ -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) {
Expand All @@ -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();
Expand Down Expand Up @@ -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) {
Expand Down
49 changes: 2 additions & 47 deletions compiler/rustc_span/src/source_map/tests.rs
Original file line number Diff line number Diff line change
@@ -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());
Expand Down Expand Up @@ -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()
Expand Down
1 change: 1 addition & 0 deletions library/core/tests/macros.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#[allow(dead_code)]
trait Trait {
fn blah(&self);
}
Expand Down
1 change: 1 addition & 0 deletions src/bootstrap/src/core/build_steps/setup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ static SETTINGS_HASHES: &[&str] = &[
"3468fea433c25fff60be6b71e8a215a732a7b1268b6a83bf10d024344e140541",
"47d227f424bf889b0d899b9cc992d5695e1b78c406e183cd78eafefbe5488923",
"b526bd58d0262dd4dda2bff5bc5515b705fb668a46235ace3e057f807963a11a",
"828666b021d837a33e78d870b56d34c88a5e2c85de58b693607ec574f0c27000",
];
static RUST_ANALYZER_SETTINGS: &str = include_str!("../../../../etc/rust_analyzer_settings.json");

Expand Down
1 change: 1 addition & 0 deletions src/etc/rust_analyzer_settings.json
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
{
"git.detectSubmodulesLimit": 20,
"rust-analyzer.check.invocationLocation": "root",
"rust-analyzer.check.invocationStrategy": "once",
"rust-analyzer.check.overrideCommand": [
Expand Down
31 changes: 19 additions & 12 deletions src/librustdoc/passes/stripper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 => {
Expand All @@ -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));
Expand Down Expand Up @@ -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;
}
}
Expand All @@ -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;
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/tools/clippy/clippy_lints/src/derive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Loading
Loading