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

Infrastructure for lints during attribute parsing, specifically duplicate usages of attributes #138164

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions compiler/rustc_ast_lowering/src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,12 +74,13 @@ impl<'hir> LoweringContext<'_, 'hir> {
// Merge attributes into the inner expression.
if !e.attrs.is_empty() {
let old_attrs = self.attrs.get(&ex.hir_id.local_id).copied().unwrap_or(&[]);
let new_attrs = self.lower_attrs_vec(&e.attrs, e.span, ex.hir_id)
.into_iter()
.chain(old_attrs.iter().cloned());
self.attrs.insert(
ex.hir_id.local_id,
&*self.arena.alloc_from_iter(
self.lower_attrs_vec(&e.attrs, e.span)
.into_iter()
.chain(old_attrs.iter().cloned()),
new_attrs,
),
);
}
Expand Down Expand Up @@ -2025,7 +2026,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
let ret_expr = self.checked_return(Some(from_residual_expr));
self.arena.alloc(self.expr(try_span, ret_expr))
};
self.lower_attrs(ret_expr.hir_id, &attrs, ret_expr.span);
self.lower_attrs(ret_expr.hir_id, &attrs, span);

let break_pat = self.pat_cf_break(try_span, residual_local);
self.arm(break_pat, ret_expr)
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_ast_lowering/src/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ impl<'a, 'hir> ItemLowerer<'a, 'hir> {
&mut self,
owner: NodeId,
f: impl FnOnce(&mut LoweringContext<'_, 'hir>) -> hir::OwnerNode<'hir>,
) {
) {
let mut lctx = LoweringContext::new(self.tcx, self.resolver);
lctx.with_hir_id_owner(owner, |lctx| f(lctx));

Expand Down
53 changes: 41 additions & 12 deletions compiler/rustc_ast_lowering/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
use rustc_data_structures::tagged_ptr::TaggedRef;
use rustc_errors::{DiagArgFromDisplay, DiagCtxtHandle, StashKey};
use rustc_hir::def::{DefKind, LifetimeRes, Namespace, PartialRes, PerNS, Res};
use rustc_hir::def_id::{CRATE_DEF_ID, LOCAL_CRATE, LocalDefId};
use rustc_hir::def_id::{LocalDefId, CRATE_DEF_ID, LOCAL_CRATE};
use rustc_hir::{
self as hir, ConstArg, GenericArg, HirId, ItemLocalMap, LangItem, ParamName, TraitCandidate,
};
Expand Down Expand Up @@ -188,7 +188,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
// interact with `gen`/`async gen` blocks
allow_async_iterator: [sym::gen_future, sym::async_iterator].into(),

attribute_parser: AttributeParser::new(tcx.sess, tcx.features(), registered_tools),
attribute_parser: AttributeParser::new(tcx, tcx.features(), registered_tools),
}
}

Expand All @@ -197,6 +197,22 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
}
}

struct SpanLowerer {
is_incremental: bool,
defid: LocalDefId,
}

impl SpanLowerer {
fn lower(&self, span: Span) -> Span {
if self.is_incremental {
span.with_parent(Some(self.defid))
} else {
// Do not make spans relative when not using incremental compilation.
span
}
}
}

#[extension(trait ResolverAstLoweringExt)]
impl ResolverAstLowering {
fn legacy_const_generic_args(&self, expr: &Expr) -> Option<Vec<usize>> {
Expand Down Expand Up @@ -740,15 +756,17 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
})
}

fn span_lowerer(&self) -> SpanLowerer {
SpanLowerer {
is_incremental: self.tcx.sess.opts.incremental.is_some(),
defid: self.current_hir_id_owner.def_id,
}
}

/// Intercept all spans entering HIR.
/// Mark a span as relative to the current owning item.
fn lower_span(&self, span: Span) -> Span {
if self.tcx.sess.opts.incremental.is_some() {
span.with_parent(Some(self.current_hir_id_owner.def_id))
} else {
// Do not make spans relative when not using incremental compilation.
span
}
self.span_lowerer().lower(span)
}

fn lower_ident(&self, ident: Ident) -> Ident {
Expand Down Expand Up @@ -871,7 +889,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
if attrs.is_empty() {
&[]
} else {
let lowered_attrs = self.lower_attrs_vec(attrs, self.lower_span(target_span));
let lowered_attrs = self.lower_attrs_vec(attrs, self.lower_span(target_span), id);

debug_assert_eq!(id.owner, self.current_hir_id_owner);
let ret = self.arena.alloc_from_iter(lowered_attrs);
Expand All @@ -891,9 +909,20 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
}
}

fn lower_attrs_vec(&self, attrs: &[Attribute], target_span: Span) -> Vec<hir::Attribute> {
self.attribute_parser
.parse_attribute_list(attrs, target_span, OmitDoc::Lower, |s| self.lower_span(s))
fn lower_attrs_vec(
&mut self,
attrs: &[Attribute],
target_span: Span,
target_hir_id: HirId,
) -> Vec<hir::Attribute> {
let l = self.span_lowerer();
self.attribute_parser.parse_attribute_list(
attrs,
target_span,
target_hir_id,
OmitDoc::Lower,
|s| l.lower(s),
)
}

fn alias_attrs(&mut self, id: HirId, target_id: HirId) {
Expand Down
10 changes: 10 additions & 0 deletions compiler/rustc_attr_parsing/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,17 @@ attr_parsing_unsupported_literal_generic =
attr_parsing_unsupported_literal_suggestion =
consider removing the prefix

attr_parsing_unused_duplicate =
unused attribute
.suggestion = remove this attribute
.note = attribute also specified here
.warn = {-attr_parsing_previously_accepted}


attr_parsing_unused_multiple =
multiple `{$name}` attributes
.suggestion = remove this attribute
.note = attribute also specified here

-attr_parsing_perviously_accepted =
this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
34 changes: 18 additions & 16 deletions compiler/rustc_attr_parsing/src/attributes/allow_unstable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,41 +4,43 @@ use rustc_attr_data_structures::AttributeKind;
use rustc_span::{Span, Symbol, sym};

use super::{CombineAttributeParser, ConvertFn};
use crate::context::AcceptContext;
use crate::context::{AcceptContext, Stage};
use crate::parser::ArgParser;
use crate::session_diagnostics;

pub(crate) struct AllowInternalUnstableParser;
impl CombineAttributeParser for AllowInternalUnstableParser {
impl<S: Stage> CombineAttributeParser<S> for AllowInternalUnstableParser {
const PATH: &'static [rustc_span::Symbol] = &[sym::allow_internal_unstable];
type Item = (Symbol, Span);
const CONVERT: ConvertFn<Self::Item> = AttributeKind::AllowInternalUnstable;

fn extend<'a>(
cx: &'a AcceptContext<'a>,
args: &'a ArgParser<'a>,
) -> impl IntoIterator<Item = Self::Item> + 'a {
parse_unstable(cx, args, Self::PATH[0]).into_iter().zip(iter::repeat(cx.attr_span))
fn extend<'c>(
cx: &'c mut AcceptContext<'_, '_, S>,
args: &'c ArgParser<'_>,
) -> impl IntoIterator<Item = Self::Item> {
parse_unstable(cx, args, <Self as CombineAttributeParser<S>>::PATH[0])
.into_iter()
.zip(iter::repeat(cx.attr_span))
}
}

pub(crate) struct AllowConstFnUnstableParser;
impl CombineAttributeParser for AllowConstFnUnstableParser {
impl<S: Stage> CombineAttributeParser<S> for AllowConstFnUnstableParser {
const PATH: &'static [rustc_span::Symbol] = &[sym::rustc_allow_const_fn_unstable];
type Item = Symbol;
const CONVERT: ConvertFn<Self::Item> = AttributeKind::AllowConstFnUnstable;

fn extend<'a>(
cx: &'a AcceptContext<'a>,
args: &'a ArgParser<'a>,
) -> impl IntoIterator<Item = Self::Item> + 'a {
parse_unstable(cx, args, Self::PATH[0])
fn extend<'c>(
cx: &'c mut AcceptContext<'_, '_, S>,
args: &'c ArgParser<'_>,
) -> impl IntoIterator<Item = Self::Item> + 'c {
parse_unstable(cx, args, <Self as CombineAttributeParser<S>>::PATH[0])
}
}

fn parse_unstable<'a>(
cx: &AcceptContext<'_>,
args: &'a ArgParser<'a>,
fn parse_unstable<S: Stage>(
cx: &AcceptContext<'_, '_, S>,
args: &ArgParser<'_>,
symbol: Symbol,
) -> impl IntoIterator<Item = Symbol> {
let mut res = Vec::new();
Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_attr_parsing/src/attributes/confusables.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use rustc_span::{Span, Symbol, sym};
use thin_vec::ThinVec;

use super::{AcceptMapping, AttributeParser};
use crate::context::FinalizeContext;
use crate::context::{FinalizeContext, Stage};
use crate::session_diagnostics;

#[derive(Default)]
Expand All @@ -12,8 +12,8 @@ pub(crate) struct ConfusablesParser {
first_span: Option<Span>,
}

impl AttributeParser for ConfusablesParser {
const ATTRIBUTES: AcceptMapping<Self> = &[(&[sym::rustc_confusables], |this, cx, args| {
impl<S: Stage> AttributeParser<S> for ConfusablesParser {
const ATTRIBUTES: AcceptMapping<Self, S> = &[(&[sym::rustc_confusables], |this, cx, args| {
let Some(list) = args.list() else {
// FIXME(jdonszelmann): error when not a list? Bring validation code here.
// NOTE: currently subsequent attributes are silently ignored using
Expand Down Expand Up @@ -45,7 +45,7 @@ impl AttributeParser for ConfusablesParser {
this.first_span.get_or_insert(cx.attr_span);
})];

fn finalize(self, _cx: &FinalizeContext<'_>) -> Option<AttributeKind> {
fn finalize(self, _cx: &FinalizeContext<'_, '_, S>) -> Option<AttributeKind> {
if self.confusables.is_empty() {
return None;
}
Expand Down
23 changes: 8 additions & 15 deletions compiler/rustc_attr_parsing/src/attributes/deprecation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,17 @@ use rustc_attr_data_structures::{AttributeKind, DeprecatedSince, Deprecation};
use rustc_span::symbol::Ident;
use rustc_span::{Span, Symbol, sym};

use super::SingleAttributeParser;
use super::util::parse_version;
use crate::context::AcceptContext;
use super::{AttributeOrder, OnDuplicate, SingleAttributeParser};
use crate::context::{AcceptContext, Stage};
use crate::parser::ArgParser;
use crate::session_diagnostics;
use crate::session_diagnostics::UnsupportedLiteralReason;

pub(crate) struct DeprecationParser;

fn get(
cx: &AcceptContext<'_>,
fn get<S: Stage>(
cx: &AcceptContext<'_, '_, S>,
ident: Ident,
param_span: Span,
arg: &ArgParser<'_>,
Expand Down Expand Up @@ -45,19 +45,12 @@ fn get(
}
}

impl SingleAttributeParser for DeprecationParser {
impl<S: Stage> SingleAttributeParser<S> for DeprecationParser {
const PATH: &'static [rustc_span::Symbol] = &[sym::deprecated];
const ATTRIBUTE_ORDER: AttributeOrder = AttributeOrder::KeepFirst;
const ON_DUPLICATE: OnDuplicate<S> = OnDuplicate::Error;

fn on_duplicate(cx: &AcceptContext<'_>, first_span: rustc_span::Span) {
// FIXME(jdonszelmann): merge with errors from check_attrs.rs
cx.emit_err(session_diagnostics::UnusedMultiple {
this: cx.attr_span,
other: first_span,
name: sym::deprecated,
});
}

fn convert(cx: &AcceptContext<'_>, args: &ArgParser<'_>) -> Option<AttributeKind> {
fn convert(cx: &AcceptContext<'_, '_, S>, args: &ArgParser<'_>) -> Option<AttributeKind> {
let features = cx.features();

let mut since = None;
Expand Down
Loading