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.

Sign up for GitHub

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

Jump to bottom

Validate proc macro attributes in AST validation #108859

Closed
wants to merge 2 commits into from
Closed

Validate proc macro attributes in AST validation #108859

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
14044f2
Check crate-types when creating it.
cjgillot Mar 6, 2023
54dfda9
Check proc-macro attributes in ast validation.
cjgillot Mar 6, 2023
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
78 changes: 69 additions & 9 deletions compiler/rustc_ast_passes/src/ast_validation.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ use rustc_ast_pretty::pprust::{self, State};
use rustc_data_structures::fx::FxHashMap;
use rustc_macros::Subdiagnostic;
use rustc_parse::validate_attr;
use rustc_session::config::CrateType;
use rustc_session::lint::builtin::{
DEPRECATED_WHERE_CLAUSE_LOCATION, MISSING_ABI, PATTERNS_IN_FNS_WITHOUT_BODY,
};
Expand Down Expand Up @@ -45,6 +46,7 @@ enum DisallowTildeConstContext<'a> {

struct AstValidator<'a> {
session: &'a Session,
is_proc_macro_crate: bool,

/// The span of the `extern` in an `extern { ... }` block, if any.
extern_mod: Option<&'a Item>,
Expand All @@ -54,8 +56,6 @@ struct AstValidator<'a> {

in_const_trait_impl: bool,

has_proc_macro_decls: bool,

/// Used to ban nested `impl Trait`, e.g., `impl Into<impl Debug>`.
/// Nested `impl Trait` _is_ allowed in associated type position,
/// e.g., `impl Iterator<Item = impl Debug>`.
Expand Down Expand Up @@ -633,6 +633,69 @@ impl<'a> AstValidator<'a> {
)
}
}

fn check_proc_macro_attr(&mut self, item: &Item) {
let mut found_attr: Option<&Attribute> = None;
for attr in &item.attrs {
if self.session.is_proc_macro_attr(&attr) {
if let Some(prev_attr) = found_attr {
let prev_item = prev_attr.get_normal_item();
let item = attr.get_normal_item();
let path_str = pprust::path_to_string(&item.path);
let msg = if item.path.segments[0].ident.name
== prev_item.path.segments[0].ident.name
{
format!(
"only one `#[{}]` attribute is allowed on any given function",
path_str,
)
} else {
format!(
"`#[{}]` and `#[{}]` attributes cannot both be applied
to the same function",
path_str,
pprust::path_to_string(&prev_item.path),
)
};

self.session
.struct_span_err(attr.span, &msg)
.span_label(prev_attr.span, "previous attribute here")
.emit();

return;
}

found_attr = Some(attr);
}
}

let Some(attr) = found_attr else { return };

if !matches!(item.kind, ast::ItemKind::Fn(..)) {
let msg = format!(
"the `#[{}]` attribute may only be used on bare functions",
pprust::path_to_string(&attr.get_normal_item().path),
);

self.session.span_err(attr.span, &msg);
return;
}

if self.session.opts.test {
return;
}

if !self.is_proc_macro_crate {
let msg = format!(
"the `#[{}]` attribute is only usable with crates of the `proc-macro` crate type",
pprust::path_to_string(&attr.get_normal_item().path),
);

self.session.span_err(attr.span, &msg);
return;
}
}
}

/// Checks that generic parameters are in the correct order,
Expand Down Expand Up @@ -799,9 +862,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
}

fn visit_item(&mut self, item: &'a Item) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, what happens if the proc-macro attributes are put on non-items?

if item.attrs.iter().any(|attr| self.session.is_proc_macro_attr(attr)) {
self.has_proc_macro_decls = true;
}
self.check_proc_macro_attr(item);

if self.session.contains_name(&item.attrs, sym::no_mangle) {
self.check_nomangle_item_asciionly(item.ident, item.span);
Expand Down Expand Up @@ -1463,22 +1524,21 @@ fn deny_equality_constraints(
this.err_handler().emit_err(err);
}

pub fn check_crate(session: &Session, krate: &Crate, lints: &mut LintBuffer) -> bool {
pub fn check_crate(session: &Session, krate: &Crate, lints: &mut LintBuffer) {
let is_proc_macro_crate = session.crate_types().contains(&CrateType::ProcMacro);
let mut validator = AstValidator {
session,
extern_mod: None,
in_trait_impl: false,
in_const_trait_impl: false,
has_proc_macro_decls: false,
is_proc_macro_crate,
outer_impl_trait: None,
disallow_tilde_const: None,
is_impl_trait_banned: false,
forbidden_let_reason: Some(ForbiddenLetReason::GenericForbidden),
lint_buffer: lints,
};
visit::walk_crate(&mut validator, krate);

validator.has_proc_macro_decls
}

/// Used to forbid `let` expressions in certain syntactic locations.
Expand Down
2 changes: 0 additions & 2 deletions compiler/rustc_ast_passes/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@
#![feature(iter_is_partitioned)]
#![feature(let_chains)]
#![recursion_limit = "256"]
#![deny(rustc::untranslatable_diagnostic)]
#![deny(rustc::diagnostic_outside_of_impl)]

use rustc_errors::{DiagnosticMessage, SubdiagnosticMessage};
use rustc_macros::fluent_messages;
Expand Down
89 changes: 7 additions & 82 deletions compiler/rustc_builtin_macros/src/proc_macro_harness.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
use rustc_ast::ptr::P;
use rustc_ast::visit::{self, Visitor};
use rustc_ast::{self as ast, NodeId};
use rustc_ast_pretty::pprust;
use rustc_expand::base::{parse_macro_name_and_helper_attrs, ExtCtxt, ResolverExpand};
use rustc_expand::expand::{AstFragment, ExpansionConfig};
use rustc_session::Session;
Expand Down Expand Up @@ -39,18 +38,12 @@ struct CollectProcMacros<'a> {
in_root: bool,
handler: &'a rustc_errors::Handler,
source_map: &'a SourceMap,
is_proc_macro_crate: bool,
is_test_crate: bool,
}

pub fn inject(
sess: &Session,
resolver: &mut dyn ResolverExpand,
mut krate: ast::Crate,
is_proc_macro_crate: bool,
has_proc_macro_decls: bool,
is_test_crate: bool,
handler: &rustc_errors::Handler,
) -> ast::Crate {
let ecfg = ExpansionConfig::default("proc_macro".to_string());
let mut cx = ExtCtxt::new(sess, ecfg, resolver, None);
Expand All @@ -59,22 +52,14 @@ pub fn inject(
sess,
macros: Vec::new(),
in_root: true,
handler,
handler: sess.diagnostic(),
source_map: sess.source_map(),
is_proc_macro_crate,
is_test_crate,
};

if has_proc_macro_decls || is_proc_macro_crate {
visit::walk_crate(&mut collect, &krate);
}
visit::walk_crate(&mut collect, &krate);
let macros = collect.macros;

if !is_proc_macro_crate {
return krate;
}

if is_test_crate {
if sess.opts.test {
return krate;
}

Expand All @@ -86,7 +71,7 @@ pub fn inject(

impl<'a> CollectProcMacros<'a> {
fn check_not_pub_in_root(&self, vis: &ast::Visibility, sp: Span) {
if self.is_proc_macro_crate && self.in_root && vis.kind.is_pub() {
if self.in_root && vis.kind.is_pub() {
self.handler.span_err(
sp,
"`proc-macro` crate types currently cannot export any items other \
Expand Down Expand Up @@ -160,54 +145,14 @@ impl<'a> CollectProcMacros<'a> {
impl<'a> Visitor<'a> for CollectProcMacros<'a> {
fn visit_item(&mut self, item: &'a ast::Item) {
if let ast::ItemKind::MacroDef(..) = item.kind {
if self.is_proc_macro_crate && self.sess.contains_name(&item.attrs, sym::macro_export) {
if self.sess.contains_name(&item.attrs, sym::macro_export) {
let msg =
"cannot export macro_rules! macros from a `proc-macro` crate type currently";
self.handler.span_err(self.source_map.guess_head_span(item.span), msg);
}
}

// First up, make sure we're checking a bare function. If we're not then
// we're just not interested in this item.
//
// If we find one, try to locate a `#[proc_macro_derive]` attribute on it.
let is_fn = matches!(item.kind, ast::ItemKind::Fn(..));

let mut found_attr: Option<&'a ast::Attribute> = None;

for attr in &item.attrs {
if self.sess.is_proc_macro_attr(&attr) {
if let Some(prev_attr) = found_attr {
let prev_item = prev_attr.get_normal_item();
let item = attr.get_normal_item();
let path_str = pprust::path_to_string(&item.path);
let msg = if item.path.segments[0].ident.name
== prev_item.path.segments[0].ident.name
{
format!(
"only one `#[{}]` attribute is allowed on any given function",
path_str,
)
} else {
format!(
"`#[{}]` and `#[{}]` attributes cannot both be applied
to the same function",
path_str,
pprust::path_to_string(&prev_item.path),
)
};

self.handler
.struct_span_err(attr.span, &msg)
.span_label(prev_attr.span, "previous attribute here")
.emit();

return;
}

found_attr = Some(attr);
}
}
let found_attr = item.attrs.iter().find(|attr| self.sess.is_proc_macro_attr(&attr));

let Some(attr) = found_attr else {
self.check_not_pub_in_root(&item.vis, self.source_map.guess_head_span(item.span));
Expand All @@ -217,27 +162,7 @@ impl<'a> Visitor<'a> for CollectProcMacros<'a> {
return;
};

if !is_fn {
let msg = format!(
"the `#[{}]` attribute may only be used on bare functions",
pprust::path_to_string(&attr.get_normal_item().path),
);

self.handler.span_err(attr.span, &msg);
return;
}

if self.is_test_crate {
return;
}

if !self.is_proc_macro_crate {
let msg = format!(
"the `#[{}]` attribute is only usable with crates of the `proc-macro` crate type",
pprust::path_to_string(&attr.get_normal_item().path),
);

self.handler.span_err(attr.span, &msg);
if self.sess.opts.test {
return;
}

Expand Down
48 changes: 20 additions & 28 deletions compiler/rustc_interface/src/passes.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,22 @@ pub fn register_plugins<'a>(
sess.init_features(features);

let crate_types = util::collect_crate_types(sess, &krate.attrs);
let is_executable_crate = crate_types.contains(&CrateType::Executable);
let is_proc_macro_crate = crate_types.contains(&CrateType::ProcMacro);
if crate_types.len() > 1 {
if is_executable_crate {
sess.emit_err(errors::MixedBinCrate);
}
if is_proc_macro_crate {
sess.emit_err(errors::MixedProcMacroCrate);
}
}
sess.init_crate_types(crate_types);

if is_proc_macro_crate && sess.panic_strategy() == PanicStrategy::Abort {
sess.emit_warning(errors::ProcMacroCratePanicAbort);
}

let stable_crate_id = StableCrateId::new(
crate_name,
sess.crate_types().contains(&CrateType::Executable),
Expand Down Expand Up @@ -266,40 +280,18 @@ fn configure_and_expand(mut krate: ast::Crate, resolver: &mut Resolver<'_, '_>)
rustc_builtin_macros::test_harness::inject(sess, resolver, &mut krate)
});

let has_proc_macro_decls = sess.time("AST_validation", || {
sess.time("AST_validation", || {
rustc_ast_passes::ast_validation::check_crate(sess, &krate, resolver.lint_buffer())
});

let crate_types = sess.crate_types();
let is_executable_crate = crate_types.contains(&CrateType::Executable);
let is_proc_macro_crate = crate_types.contains(&CrateType::ProcMacro);
let is_proc_macro_crate = sess.crate_types().contains(&CrateType::ProcMacro);

if crate_types.len() > 1 {
if is_executable_crate {
sess.emit_err(errors::MixedBinCrate);
}
if is_proc_macro_crate {
sess.emit_err(errors::MixedProcMacroCrate);
}
}

if is_proc_macro_crate && sess.panic_strategy() == PanicStrategy::Abort {
sess.emit_warning(errors::ProcMacroCratePanicAbort);
if is_proc_macro_crate {
krate = sess.time("maybe_create_a_macro_crate", || {
rustc_builtin_macros::proc_macro_harness::inject(sess, resolver, krate)
});
}

krate = sess.time("maybe_create_a_macro_crate", || {
let is_test_crate = sess.opts.test;
rustc_builtin_macros::proc_macro_harness::inject(
sess,
resolver,
krate,
is_proc_macro_crate,
has_proc_macro_decls,
is_test_crate,
sess.diagnostic(),
)
});

// Done with macro expansion!

if sess.opts.unstable_opts.input_stats {
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/feature-gates/issue-43106-gating-of-proc_macro_derive.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
//~^ ERROR the `#[proc_macro_derive]` attribute may only be used on bare functions
mod proc_macro_derive1 {
mod inner { #![proc_macro_derive()] }
// (no error issued here if there was one on outer module)
//~^ ERROR the `#[proc_macro_derive]` attribute may only be used on bare functions
}

mod proc_macro_derive2 {
Expand Down
8 changes: 7 additions & 1 deletion tests/ui/feature-gates/issue-43106-gating-of-proc_macro_derive.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,12 @@ error: the `#[proc_macro_derive]` attribute may only be used on bare functions
LL | #[proc_macro_derive()]
| ^^^^^^^^^^^^^^^^^^^^^^

error: the `#[proc_macro_derive]` attribute may only be used on bare functions
--> $DIR/issue-43106-gating-of-proc_macro_derive.rs:13:17
|
LL | mod inner { #![proc_macro_derive()] }
| ^^^^^^^^^^^^^^^^^^^^^^^

error: the `#[proc_macro_derive]` attribute may only be used on bare functions
--> $DIR/issue-43106-gating-of-proc_macro_derive.rs:18:17
|
Expand Down Expand Up @@ -34,5 +40,5 @@ error: the `#[proc_macro_derive]` attribute may only be used on bare functions
LL | #[proc_macro_derive()] impl S { }
| ^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to 6 previous errors
error: aborting due to 7 previous errors