Skip to content

Move attribute validation from rustc_parse to rustc_attr_parsing #143963

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
2 changes: 1 addition & 1 deletion Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -3275,7 +3275,6 @@ dependencies = [
"rustc_feature",
"rustc_fluent_macro",
"rustc_macros",
"rustc_parse",
"rustc_session",
"rustc_span",
"rustc_target",
Expand Down Expand Up @@ -3321,6 +3320,7 @@ dependencies = [
"rustc_hir",
"rustc_lexer",
"rustc_macros",
"rustc_parse",
"rustc_session",
"rustc_span",
"thin-vec",
Expand Down
1 change: 0 additions & 1 deletion compiler/rustc_ast_passes/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ rustc_errors = { path = "../rustc_errors" }
rustc_feature = { path = "../rustc_feature" }
rustc_fluent_macro = { path = "../rustc_fluent_macro" }
rustc_macros = { path = "../rustc_macros" }
rustc_parse = { path = "../rustc_parse" }
rustc_session = { path = "../rustc_session" }
rustc_span = { path = "../rustc_span" }
rustc_target = { path = "../rustc_target" }
Expand Down
5 changes: 0 additions & 5 deletions compiler/rustc_ast_passes/src/ast_validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ use rustc_ast_pretty::pprust::{self, State};
use rustc_data_structures::fx::FxIndexMap;
use rustc_errors::DiagCtxtHandle;
use rustc_feature::Features;
use rustc_parse::validate_attr;
use rustc_session::Session;
use rustc_session::lint::builtin::{
DEPRECATED_WHERE_CLAUSE_LOCATION, MISSING_ABI, MISSING_UNSAFE_ON_EXTERN,
Expand Down Expand Up @@ -928,10 +927,6 @@ fn validate_generic_param_order(dcx: DiagCtxtHandle<'_>, generics: &[GenericPara
}

impl<'a> Visitor<'a> for AstValidator<'a> {
fn visit_attribute(&mut self, attr: &Attribute) {
validate_attr::check_attr(&self.sess.psess, attr, self.lint_node_id);
}

fn visit_ty(&mut self, ty: &'a Ty) {
self.visit_ty_common(ty);
self.walk_ty(ty)
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_attr_data_structures/src/lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,5 @@ pub enum AttributeLintKind {
UnusedDuplicate { this: Span, other: Span, warning: bool },
IllFormedAttributeInput { suggestions: Vec<String> },
EmptyAttribute { first_span: Span },
UnsafeAttrOutsideUnsafe { attribute_name_span: Span, sugg_spans: (Span, Span) },
}
1 change: 1 addition & 0 deletions compiler/rustc_attr_parsing/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ rustc_fluent_macro = { path = "../rustc_fluent_macro" }
rustc_hir = { path = "../rustc_hir" }
rustc_lexer = { path = "../rustc_lexer" }
rustc_macros = { path = "../rustc_macros" }
rustc_parse = { path = "../rustc_parse" }
rustc_session = { path = "../rustc_session" }
rustc_span = { path = "../rustc_span" }
thin-vec = "0.2.12"
Expand Down
9 changes: 9 additions & 0 deletions compiler/rustc_attr_parsing/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -161,3 +161,12 @@ attr_parsing_unused_multiple =
-attr_parsing_previously_accepted =
this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
attr_parsing_unsafe_attr_outside_unsafe = unsafe attribute used without unsafe
.label = usage of unsafe attribute
attr_parsing_unsafe_attr_outside_unsafe_suggestion = wrap the attribute in `unsafe(...)`
attr_parsing_invalid_attr_unsafe = `{$name}` is not an unsafe attribute
.label = this is not an unsafe attribute
.suggestion = remove the `unsafe(...)`
.note = extraneous unsafe is not allowed in attributes
15 changes: 13 additions & 2 deletions compiler/rustc_attr_parsing/src/attributes/stability.rs
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,12 @@ pub(crate) fn parse_stability<S: Stage>(
let mut feature = None;
let mut since = None;

for param in args.list()?.mixed() {
let ArgParser::List(list) = args else {
cx.expected_list(cx.attr_span);
return None;
};

for param in list.mixed() {
let param_span = param.span();
let Some(param) = param.meta_item() else {
cx.emit_err(session_diagnostics::UnsupportedLiteral {
Expand Down Expand Up @@ -322,7 +327,13 @@ pub(crate) fn parse_unstability<S: Stage>(
let mut is_soft = false;
let mut implied_by = None;
let mut old_name = None;
for param in args.list()?.mixed() {

let ArgParser::List(list) = args else {
cx.expected_list(cx.attr_span);
return None;
};

for param in list.mixed() {
let Some(param) = param.meta_item() else {
cx.emit_err(session_diagnostics::UnsupportedLiteral {
span: param.span(),
Expand Down
41 changes: 36 additions & 5 deletions compiler/rustc_attr_parsing/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -203,10 +203,11 @@ mod private {
#[allow(private_interfaces)]
pub trait Stage: Sized + 'static + Sealed {
type Id: Copy;
const SHOULD_EMIT_LINTS: bool;

fn parsers() -> &'static group_type!(Self);

fn should_emit_errors_and_lints(&self) -> bool;

fn emit_err<'sess>(
&self,
sess: &'sess Session,
Expand All @@ -218,11 +219,15 @@ pub trait Stage: Sized + 'static + Sealed {
#[allow(private_interfaces)]
impl Stage for Early {
type Id = NodeId;
const SHOULD_EMIT_LINTS: bool = false;

fn parsers() -> &'static group_type!(Self) {
&early::ATTRIBUTE_PARSERS
}

fn should_emit_errors_and_lints(&self) -> bool {
self.emit_errors.should_emit()
}

fn emit_err<'sess>(
&self,
sess: &'sess Session,
Expand All @@ -240,11 +245,15 @@ impl Stage for Early {
#[allow(private_interfaces)]
impl Stage for Late {
type Id = HirId;
const SHOULD_EMIT_LINTS: bool = true;

fn parsers() -> &'static group_type!(Self) {
&late::ATTRIBUTE_PARSERS
}

fn should_emit_errors_and_lints(&self) -> bool {
true
}

fn emit_err<'sess>(
&self,
tcx: &'sess Session,
Expand Down Expand Up @@ -290,7 +299,7 @@ impl<'f, 'sess: 'f, S: Stage> SharedContext<'f, 'sess, S> {
/// must be delayed until after HIR is built. This method will take care of the details of
/// that.
pub(crate) fn emit_lint(&mut self, lint: AttributeLintKind, span: Span) {
if !S::SHOULD_EMIT_LINTS {
if !self.stage.should_emit_errors_and_lints() {
return;
}
let id = self.target_id;
Expand Down Expand Up @@ -696,6 +705,22 @@ impl<'sess> AttributeParser<'sess, Early> {
};
parse_fn(&mut cx, args)
}

/// Returns whether the attribute is valid
pub fn validate_attribute_early(
sess: &'sess Session,
attr: &ast::Attribute,
target_node_id: NodeId,
) -> bool {
let parser = Self {
features: None,
tools: Vec::new(),
parse_only: None,
sess,
stage: Early { emit_errors: ShouldEmit::ErrorsAndLints },
};
parser.validate_attribute(attr, target_node_id, &mut |_| {}, true)
}
}

impl<'sess, S: Stage> AttributeParser<'sess, S> {
Expand All @@ -712,6 +737,10 @@ impl<'sess, S: Stage> AttributeParser<'sess, S> {
&self.sess
}

pub(crate) fn stage(&self) -> &S {
&self.stage
}

pub(crate) fn features(&self) -> &'sess Features {
self.features.expect("features not available at this point in the compiler")
}
Expand Down Expand Up @@ -785,12 +814,14 @@ impl<'sess, S: Stage> AttributeParser<'sess, S> {
ast::AttrKind::Normal(n) => {
attr_paths.push(PathParser::Ast(&n.item.path));

// Parse attribute using new infra
let parser = MetaItemParser::from_attr(n, self.dcx());
let path = parser.path();
let args = parser.args();
let parts = path.segments().map(|i| i.name).collect::<Vec<_>>();

if let Some(accepts) = S::parsers().0.get(parts.as_slice()) {
self.validate_attribute(attr, target_id, &mut emit_lint, true);
for (template, accept) in accepts {
let mut cx: AcceptContext<'_, 'sess, S> = AcceptContext {
shared: SharedContext {
Expand Down Expand Up @@ -821,7 +852,7 @@ impl<'sess, S: Stage> AttributeParser<'sess, S> {
// // || FIXME_TEMPORARY_ATTR_ALLOWLIST.contains(&parts[0]),
// "attribute {path} wasn't parsed and isn't a know tool attribute",
// );

self.validate_attribute(attr, target_id, &mut emit_lint, false);
attributes.push(Attribute::Unparsed(Box::new(AttrItem {
path: AttrPath::from_ast(&n.item.path),
args: self.lower_attr_args(&n.item.args, lower_span),
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_attr_parsing/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ pub(crate) mod context;
mod lints;
pub mod parser;
mod session_diagnostics;
mod validate_attr;

pub use attributes::cfg::{CFG_TEMPLATE, EvalConfigResult, eval_config_entry, parse_cfg_attr};
pub use attributes::cfg_old::*;
Expand All @@ -97,5 +98,8 @@ pub use attributes::util::{
};
pub use context::{AttributeParser, Early, Late, OmitDoc, ShouldEmit};
pub use lints::emit_attribute_lint;
pub use validate_attr::{
check_attribute_safety, check_builtin_meta_item, emit_fatal_malformed_builtin_attribute,
};

rustc_fluent_macro::fluent_messages! { "../messages.ftl" }
15 changes: 15 additions & 0 deletions compiler/rustc_attr_parsing/src/lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use rustc_errors::{DiagArgValue, LintEmitter};
use rustc_hir::HirId;

use crate::session_diagnostics;
use crate::session_diagnostics::{UnsafeAttrOutsideUnsafeLint, UnsafeAttrOutsideUnsafeSuggestion};

pub fn emit_attribute_lint<L: LintEmitter>(lint: &AttributeLint<HirId>, lint_emitter: L) {
let AttributeLint { id, span, kind } = lint;
Expand Down Expand Up @@ -34,5 +35,19 @@ pub fn emit_attribute_lint<L: LintEmitter>(lint: &AttributeLint<HirId>, lint_emi
*first_span,
session_diagnostics::EmptyAttributeList { attr_span: *first_span },
),
&AttributeLintKind::UnsafeAttrOutsideUnsafe { attribute_name_span, sugg_spans } => {
lint_emitter.emit_node_span_lint(
rustc_session::lint::builtin::UNSAFE_ATTR_OUTSIDE_UNSAFE,
*id,
*span,
UnsafeAttrOutsideUnsafeLint {
span: attribute_name_span,
suggestion: UnsafeAttrOutsideUnsafeSuggestion {
left: sugg_spans.0.shrink_to_lo(),
right: sugg_spans.1.shrink_to_hi(),
},
},
)
}
}
}
42 changes: 42 additions & 0 deletions compiler/rustc_attr_parsing/src/session_diagnostics.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use std::num::IntErrorKind;

use rustc_ast as ast;
use rustc_ast::Path;
use rustc_errors::codes::*;
use rustc_errors::{
Applicability, Diag, DiagArgValue, DiagCtxtHandle, Diagnostic, EmissionGuarantee, Level,
Expand Down Expand Up @@ -701,3 +702,44 @@ impl<'a, G: EmissionGuarantee> Diagnostic<'a, G> for AttributeParseError {
diag
}
}

#[derive(Diagnostic)]
#[diag(attr_parsing_unsafe_attr_outside_unsafe)]
pub(crate) struct UnsafeAttrOutsideUnsafe {
#[primary_span]
#[label]
pub span: Span,
#[subdiagnostic]
pub suggestion: UnsafeAttrOutsideUnsafeSuggestion,
}

#[derive(Subdiagnostic)]
#[multipart_suggestion(
attr_parsing_unsafe_attr_outside_unsafe_suggestion,
applicability = "machine-applicable"
)]
pub(crate) struct UnsafeAttrOutsideUnsafeSuggestion {
#[suggestion_part(code = "unsafe(")]
pub left: Span,
#[suggestion_part(code = ")")]
pub right: Span,
}

#[derive(Diagnostic)]
#[diag(attr_parsing_invalid_attr_unsafe)]
#[note]
pub(crate) struct InvalidAttrUnsafe {
#[primary_span]
#[label]
pub span: Span,
pub name: Path,
}

#[derive(LintDiagnostic)]
#[diag(attr_parsing_unsafe_attr_outside_unsafe)]
pub(crate) struct UnsafeAttrOutsideUnsafeLint {
#[label]
pub span: Span,
#[subdiagnostic]
pub suggestion: UnsafeAttrOutsideUnsafeSuggestion,
}
Loading
Loading