Skip to content

Commit

Permalink
Auto merge of #95562 - lcnr:attr-no-encode, r=davidtwco
Browse files Browse the repository at this point in the history
don't encode only locally used attrs

Part of rust-lang/compiler-team#505.

We now filter builtin attributes before encoding them in the crate metadata in case they should only be used in the local crate. To prevent accidental misuse `get_attrs` now requires the caller to state which attribute they are interested in. For places where that isn't trivially possible, I've added a method `fn get_attrs_unchecked` which I intend to remove in a followup PR.

After this pull request landed, we can then slowly move all attributes to only be used in the local crate while being certain that we don't accidentally try to access them from extern crates.

cc rust-lang/rust#94963 (comment)
  • Loading branch information
bors committed May 12, 2022
2 parents 3182532 + 107ee40 commit 7161a70
Show file tree
Hide file tree
Showing 11 changed files with 27 additions and 45 deletions.
6 changes: 3 additions & 3 deletions clippy_lints/src/derivable_impls.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use clippy_utils::diagnostics::span_lint_and_help;
use clippy_utils::{is_automatically_derived, is_default_equivalent, peel_blocks};
use clippy_utils::{is_default_equivalent, peel_blocks};
use rustc_hir::{
def::{DefKind, Res},
Body, Expr, ExprKind, GenericArg, Impl, ImplItemKind, Item, ItemKind, Node, PathSegment, QPath, TyKind,
Expand Down Expand Up @@ -71,8 +71,7 @@ impl<'tcx> LateLintPass<'tcx> for DerivableImpls {
self_ty,
..
}) = item.kind;
if let attrs = cx.tcx.hir().attrs(item.hir_id());
if !is_automatically_derived(attrs);
if !cx.tcx.has_attr(item.def_id.to_def_id(), sym::automatically_derived);
if !item.span.from_expansion();
if let Some(def_id) = trait_ref.trait_def_id();
if cx.tcx.is_diagnostic_item(sym::Default, def_id);
Expand All @@ -81,6 +80,7 @@ impl<'tcx> LateLintPass<'tcx> for DerivableImpls {
if let ImplItemKind::Fn(_, b) = &impl_item.kind;
if let Body { value: func_expr, .. } = cx.tcx.hir().body(*b);
if let Some(adt_def) = cx.tcx.type_of(item.def_id).ty_adt_def();
if let attrs = cx.tcx.hir().attrs(item.hir_id());
if !attrs.iter().any(|attr| attr.doc_str().is_some());
if let child_attrs = cx.tcx.hir().attrs(impl_item_hir);
if !child_attrs.iter().any(|attr| attr.doc_str().is_some());
Expand Down
10 changes: 5 additions & 5 deletions clippy_lints/src/derive.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_note, span_lint_and_then};
use clippy_utils::paths;
use clippy_utils::ty::{implements_trait, is_copy};
use clippy_utils::{is_automatically_derived, is_lint_allowed, match_def_path};
use clippy_utils::{is_lint_allowed, match_def_path};
use if_chain::if_chain;
use rustc_hir::intravisit::{walk_expr, walk_fn, walk_item, FnKind, Visitor};
use rustc_hir::{
Expand Down Expand Up @@ -171,8 +171,8 @@ impl<'tcx> LateLintPass<'tcx> for Derive {
}) = item.kind
{
let ty = cx.tcx.type_of(item.def_id);
let attrs = cx.tcx.hir().attrs(item.hir_id());
let is_automatically_derived = is_automatically_derived(attrs);
let is_automatically_derived =
cx.tcx.has_attr(item.def_id.to_def_id(), sym::automatically_derived);

check_hash_peq(cx, item.span, trait_ref, ty, is_automatically_derived);
check_ord_partial_ord(cx, item.span, trait_ref, ty, is_automatically_derived);
Expand Down Expand Up @@ -201,7 +201,7 @@ fn check_hash_peq<'tcx>(
then {
// Look for the PartialEq implementations for `ty`
cx.tcx.for_each_relevant_impl(peq_trait_def_id, ty, |impl_id| {
let peq_is_automatically_derived = is_automatically_derived(cx.tcx.get_attrs(impl_id));
let peq_is_automatically_derived = cx.tcx.has_attr(impl_id, sym::automatically_derived);

if peq_is_automatically_derived == hash_is_automatically_derived {
return;
Expand Down Expand Up @@ -255,7 +255,7 @@ fn check_ord_partial_ord<'tcx>(
then {
// Look for the PartialOrd implementations for `ty`
cx.tcx.for_each_relevant_impl(partial_ord_trait_def_id, ty, |impl_id| {
let partial_ord_is_automatically_derived = is_automatically_derived(cx.tcx.get_attrs(impl_id));
let partial_ord_is_automatically_derived = cx.tcx.has_attr(impl_id, sym::automatically_derived);

if partial_ord_is_automatically_derived == ord_is_automatically_derived {
return;
Expand Down
8 changes: 4 additions & 4 deletions clippy_lints/src/functions/must_use.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,13 @@ use clippy_utils::attrs::is_proc_macro;
use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_then};
use clippy_utils::source::snippet_opt;
use clippy_utils::ty::is_must_use_ty;
use clippy_utils::{match_def_path, must_use_attr, return_ty, trait_ref_of_method};
use clippy_utils::{match_def_path, return_ty, trait_ref_of_method};

use super::{DOUBLE_MUST_USE, MUST_USE_CANDIDATE, MUST_USE_UNIT};

pub(super) fn check_item<'tcx>(cx: &LateContext<'tcx>, item: &'tcx hir::Item<'_>) {
let attrs = cx.tcx.hir().attrs(item.hir_id());
let attr = must_use_attr(attrs);
let attr = cx.tcx.get_attr(item.def_id.to_def_id(), sym::must_use);
if let hir::ItemKind::Fn(ref sig, _generics, ref body_id) = item.kind {
let is_public = cx.access_levels.is_exported(item.def_id);
let fn_header_span = item.span.with_hi(sig.decl.output.span().hi());
Expand All @@ -44,7 +44,7 @@ pub(super) fn check_impl_item<'tcx>(cx: &LateContext<'tcx>, item: &'tcx hir::Imp
let is_public = cx.access_levels.is_exported(item.def_id);
let fn_header_span = item.span.with_hi(sig.decl.output.span().hi());
let attrs = cx.tcx.hir().attrs(item.hir_id());
let attr = must_use_attr(attrs);
let attr = cx.tcx.get_attr(item.def_id.to_def_id(), sym::must_use);
if let Some(attr) = attr {
check_needless_must_use(cx, sig.decl, item.hir_id(), item.span, fn_header_span, attr);
} else if is_public && !is_proc_macro(cx.sess(), attrs) && trait_ref_of_method(cx, item.def_id).is_none() {
Expand All @@ -67,7 +67,7 @@ pub(super) fn check_trait_item<'tcx>(cx: &LateContext<'tcx>, item: &'tcx hir::Tr
let fn_header_span = item.span.with_hi(sig.decl.output.span().hi());

let attrs = cx.tcx.hir().attrs(item.hir_id());
let attr = must_use_attr(attrs);
let attr = cx.tcx.get_attr(item.def_id.to_def_id(), sym::must_use);
if let Some(attr) = attr {
check_needless_must_use(cx, sig.decl, item.hir_id(), item.span, fn_header_span, attr);
} else if let hir::TraitFn::Provided(eid) = *eid {
Expand Down
3 changes: 1 addition & 2 deletions clippy_lints/src/manual_non_exhaustive.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use clippy_utils::attrs::is_doc_hidden;
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::source::snippet_opt;
use clippy_utils::{is_lint_allowed, meets_msrv, msrvs};
Expand Down Expand Up @@ -161,7 +160,7 @@ impl<'tcx> LateLintPass<'tcx> for ManualNonExhaustiveEnum {
let id = cx.tcx.hir().local_def_id(v.id);
(matches!(v.data, hir::VariantData::Unit(_))
&& v.ident.as_str().starts_with('_')
&& is_doc_hidden(cx.tcx.get_attrs(id.to_def_id())))
&& cx.tcx.is_doc_hidden(id.to_def_id()))
.then(|| (id, v.span))
});
if let Some((id, span)) = iter.next()
Expand Down
3 changes: 1 addition & 2 deletions clippy_lints/src/matches/match_wild_enum.rs
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,5 @@ impl<'a> CommonPrefixSearcher<'a> {
}

fn is_hidden(cx: &LateContext<'_>, variant_def: &VariantDef) -> bool {
let attrs = cx.tcx.get_attrs(variant_def.def_id);
clippy_utils::attrs::is_doc_hidden(attrs) || clippy_utils::attrs::is_unstable(attrs)
cx.tcx.is_doc_hidden(variant_def.def_id) || cx.tcx.has_attr(variant_def.def_id, sym::unstable)
}
2 changes: 1 addition & 1 deletion clippy_lints/src/new_without_default.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ impl<'tcx> LateLintPass<'tcx> for NewWithoutDefault {
// can't be implemented for unsafe new
return;
}
if clippy_utils::is_doc_hidden(cx.tcx.hir().attrs(id)) {
if cx.tcx.is_doc_hidden(impl_item.def_id) {
// shouldn't be implemented when it is hidden in docs
return;
}
Expand Down
4 changes: 1 addition & 3 deletions clippy_lints/src/partialeq_ne_impl.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
use clippy_utils::diagnostics::span_lint_hir;
use clippy_utils::is_automatically_derived;
use if_chain::if_chain;
use rustc_hir::{Impl, Item, ItemKind};
use rustc_lint::{LateContext, LateLintPass};
Expand Down Expand Up @@ -37,8 +36,7 @@ impl<'tcx> LateLintPass<'tcx> for PartialEqNeImpl {
fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) {
if_chain! {
if let ItemKind::Impl(Impl { of_trait: Some(ref trait_ref), items: impl_items, .. }) = item.kind;
let attrs = cx.tcx.hir().attrs(item.hir_id());
if !is_automatically_derived(attrs);
if !cx.tcx.has_attr(item.def_id.to_def_id(), sym::automatically_derived);
if let Some(eq_trait) = cx.tcx.lang_items().eq_trait();
if trait_ref.path.res.def_id() == eq_trait;
then {
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/significant_drop_in_scrutinee.rs
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ impl<'a, 'tcx> SigDropHelper<'a, 'tcx> {

fn has_sig_drop_attr(&mut self, cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool {
if let Some(adt) = ty.ty_adt_def() {
if get_attr(cx.sess(), cx.tcx.get_attrs(adt.did()), "has_significant_drop").count() > 0 {
if get_attr(cx.sess(), cx.tcx.get_attrs_unchecked(adt.did()), "has_significant_drop").count() > 0 {
return true;
}
}
Expand Down
7 changes: 2 additions & 5 deletions clippy_utils/src/attrs.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use rustc_ast::{ast, attr};
use rustc_ast::ast;
use rustc_errors::Applicability;
use rustc_session::Session;
use rustc_ast::attr;
use rustc_span::sym;
use std::str::FromStr;

Expand Down Expand Up @@ -158,7 +159,3 @@ pub fn is_doc_hidden(attrs: &[ast::Attribute]) -> bool {
.any(|l| attr::list_contains_name(&l, sym::hidden))
}

/// Return true if the attributes contain `#[unstable]`
pub fn is_unstable(attrs: &[ast::Attribute]) -> bool {
attrs.iter().any(|attr| attr.has_name(sym::unstable))
}
15 changes: 2 additions & 13 deletions clippy_utils/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ use std::lazy::SyncOnceCell;
use std::sync::{Mutex, MutexGuard};

use if_chain::if_chain;
use rustc_ast::ast::{self, Attribute, LitKind};
use rustc_ast::ast::{self, LitKind};
use rustc_data_structures::fx::FxHashMap;
use rustc_data_structures::unhash::UnhashMap;
use rustc_hir as hir;
Expand Down Expand Up @@ -1472,12 +1472,6 @@ pub fn recurse_or_patterns<'tcx, F: FnMut(&'tcx Pat<'tcx>)>(pat: &'tcx Pat<'tcx>
}
}

/// Checks for the `#[automatically_derived]` attribute all `#[derive]`d
/// implementations have.
pub fn is_automatically_derived(attrs: &[ast::Attribute]) -> bool {
has_attr(attrs, sym::automatically_derived)
}

pub fn is_self(slf: &Param<'_>) -> bool {
if let PatKind::Binding(.., name, _) = slf.pat.kind {
name.name == kw::SelfLower
Expand Down Expand Up @@ -1724,11 +1718,6 @@ pub fn get_async_fn_body<'tcx>(tcx: TyCtxt<'tcx>, body: &Body<'_>) -> Option<&'t
None
}

// Finds the `#[must_use]` attribute, if any
pub fn must_use_attr(attrs: &[Attribute]) -> Option<&Attribute> {
attrs.iter().find(|a| a.has_name(sym::must_use))
}

// check if expr is calling method or function with #[must_use] attribute
pub fn is_must_use_func_call(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
let did = match expr.kind {
Expand All @@ -1745,7 +1734,7 @@ pub fn is_must_use_func_call(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
_ => None,
};

did.map_or(false, |did| must_use_attr(cx.tcx.get_attrs(did)).is_some())
did.map_or(false, |did| cx.tcx.has_attr(did, sym::must_use))
}

/// Checks if an expression represents the identity function
Expand Down
12 changes: 6 additions & 6 deletions clippy_utils/src/ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ use rustc_trait_selection::infer::InferCtxtExt;
use rustc_trait_selection::traits::query::normalize::AtExt;
use std::iter;

use crate::{match_def_path, must_use_attr, path_res, paths};
use crate::{match_def_path, path_res, paths};

// Checks if the given type implements copy.
pub fn is_copy<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool {
Expand Down Expand Up @@ -178,18 +178,18 @@ pub fn has_drop<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool {
// Returns whether the type has #[must_use] attribute
pub fn is_must_use_ty<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool {
match ty.kind() {
ty::Adt(adt, _) => must_use_attr(cx.tcx.get_attrs(adt.did())).is_some(),
ty::Foreign(ref did) => must_use_attr(cx.tcx.get_attrs(*did)).is_some(),
ty::Adt(adt, _) => cx.tcx.has_attr(adt.did(), sym::must_use),
ty::Foreign(did) => cx.tcx.has_attr(*did, sym::must_use),
ty::Slice(ty) | ty::Array(ty, _) | ty::RawPtr(ty::TypeAndMut { ty, .. }) | ty::Ref(_, ty, _) => {
// for the Array case we don't need to care for the len == 0 case
// because we don't want to lint functions returning empty arrays
is_must_use_ty(cx, *ty)
},
ty::Tuple(substs) => substs.iter().any(|ty| is_must_use_ty(cx, ty)),
ty::Opaque(ref def_id, _) => {
ty::Opaque(def_id, _) => {
for (predicate, _) in cx.tcx.explicit_item_bounds(*def_id) {
if let ty::PredicateKind::Trait(trait_predicate) = predicate.kind().skip_binder() {
if must_use_attr(cx.tcx.get_attrs(trait_predicate.trait_ref.def_id)).is_some() {
if cx.tcx.has_attr(trait_predicate.trait_ref.def_id, sym::must_use) {
return true;
}
}
Expand All @@ -199,7 +199,7 @@ pub fn is_must_use_ty<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool {
ty::Dynamic(binder, _) => {
for predicate in binder.iter() {
if let ty::ExistentialPredicate::Trait(ref trait_ref) = predicate.skip_binder() {
if must_use_attr(cx.tcx.get_attrs(trait_ref.def_id)).is_some() {
if cx.tcx.has_attr(trait_ref.def_id, sym::must_use) {
return true;
}
}
Expand Down

0 comments on commit 7161a70

Please sign in to comment.