Skip to content

Commit

Permalink
Remove the MacroVisitor pass.
Browse files Browse the repository at this point in the history 
This pass was supposed to check use of gated features before
`#[cfg]`-stripping but this was not the case since it in fact happens
after. Checks that are actually important and must be done before macro
expansion are now made where the features are actually used. Close #32648.
Also ensure that attributes on macro-generated macro invocations are
checked as well. Close #32782 and #32655.
  • Loading branch information
@LeoTestard
LeoTestard committed Apr 21, 2016
1 parent 3aa7137 commit 8b40531
Show file tree
Hide file tree
Showing 11 changed files with 207 additions and 199 deletions.
16 changes: 5 additions & 11 deletions src/librustc_driver/driver.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -512,19 +512,13 @@ pub fn phase_2_configure_and_expand(sess: &Session,
middle::recursion_limit::update_recursion_limit(sess, &krate);
});

time(time_passes, "gated macro checking", || {
sess.track_errors(|| {
let features =
syntax::feature_gate::check_crate_macros(sess.codemap(),
&sess.parse_sess.span_diagnostic,
&krate);

// these need to be set "early" so that expansion sees `quote` if enabled.
*sess.features.borrow_mut() = features;
})
// these need to be set "early" so that expansion sees `quote` if enabled.
sess.track_errors(|| {
*sess.features.borrow_mut() =
syntax::feature_gate::get_features(&sess.parse_sess.span_diagnostic,
&krate);
})?;


krate = time(time_passes, "crate injection", || {
syntax::std_inject::maybe_inject_crates_ref(krate, sess.opts.alt_std_name.clone())
});
Expand Down
39 changes: 22 additions & 17 deletions src/librustc_plugin/load.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,27 +51,32 @@ pub fn load_plugins(sess: &Session,
addl_plugins: Option<Vec<String>>) -> Vec<PluginRegistrar> {
let mut loader = PluginLoader::new(sess, cstore, crate_name);

for attr in &krate.attrs {
if !attr.check_name("plugin") {
continue;
}

let plugins = match attr.meta_item_list() {
Some(xs) => xs,
None => {
call_malformed_plugin_attribute(sess, attr.span);
// do not report any error now. since crate attributes are
// not touched by expansion, every use of plugin without
// the feature enabled will result in an error later...
if sess.features.borrow().plugin {
for attr in &krate.attrs {
if !attr.check_name("plugin") {
continue;
}
};

for plugin in plugins {
if plugin.value_str().is_some() {
call_malformed_plugin_attribute(sess, attr.span);
continue;
let plugins = match attr.meta_item_list() {
Some(xs) => xs,
None => {
call_malformed_plugin_attribute(sess, attr.span);
continue;
}
};

for plugin in plugins {
if plugin.value_str().is_some() {
call_malformed_plugin_attribute(sess, attr.span);
continue;
}

let args = plugin.meta_item_list().map(ToOwned::to_owned).unwrap_or_default();
loader.load_plugin(plugin.span, &plugin.name(), args);
}

let args = plugin.meta_item_list().map(ToOwned::to_owned).unwrap_or_default();
loader.load_plugin(plugin.span, &plugin.name(), args);
}
}

Expand Down
43 changes: 24 additions & 19 deletions src/libsyntax/ext/expand.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,26 @@ use std_inject;
use std::collections::HashSet;
use std::env;

// this function is called to detect use of feature-gated or invalid attributes
// on macro invoations since they will not be detected after macro expansion
fn check_attributes(attrs: &[ast::Attribute], fld: &MacroExpander) {
for attr in attrs.iter() {
feature_gate::check_attribute(&attr, &fld.cx.parse_sess.span_diagnostic,
&fld.cx.parse_sess.codemap(),
&fld.cx.ecfg.features.unwrap());
}
}

pub fn expand_expr(e: P<ast::Expr>, fld: &mut MacroExpander) -> P<ast::Expr> {
let expr_span = e.span;
return e.and_then(|ast::Expr {id, node, span, attrs}| match node {

// expr_mac should really be expr_ext or something; it's the
// entry-point for all syntax extensions.
ast::ExprKind::Mac(mac) => {
if let Some(ref attrs) = attrs {
check_attributes(attrs, fld);
}

// Assert that we drop any macro attributes on the floor here
drop(attrs);
Expand Down Expand Up @@ -367,6 +380,8 @@ pub fn expand_item_mac(it: P<ast::Item>,
_ => fld.cx.span_bug(it.span, "invalid item macro invocation")
});

check_attributes(&attrs, fld);

let fm = fresh_mark();
let items = {
let expanded = match fld.cx.syntax_env.find(extname) {
Expand Down Expand Up @@ -441,18 +456,6 @@ pub fn expand_item_mac(it: P<ast::Item>,
let allow_internal_unstable = attr::contains_name(&attrs,
"allow_internal_unstable");

// ensure any #[allow_internal_unstable]s are
// detected (including nested macro definitions
// etc.)
if allow_internal_unstable && !fld.cx.ecfg.enable_allow_internal_unstable() {
feature_gate::emit_feature_err(
&fld.cx.parse_sess.span_diagnostic,
"allow_internal_unstable",
span,
feature_gate::GateIssue::Language,
feature_gate::EXPLAIN_ALLOW_INTERNAL_UNSTABLE)
}

let export = attr::contains_name(&attrs, "macro_export");
let def = ast::MacroDef {
ident: ident,
Expand Down Expand Up @@ -516,6 +519,10 @@ fn expand_stmt(stmt: Stmt, fld: &mut MacroExpander) -> SmallVector<Stmt> {
_ => return expand_non_macro_stmt(stmt, fld)
};

if let Some(ref attrs) = attrs {
check_attributes(attrs, fld);
}

// Assert that we drop any macro attributes on the floor here
drop(attrs);

Expand Down Expand Up @@ -1063,7 +1070,7 @@ fn expand_impl_item(ii: ast::ImplItem, fld: &mut MacroExpander)
attrs: ii.attrs,
vis: ii.vis,
defaultness: ii.defaultness,
node: match ii.node {
node: match ii.node {
ast::ImplItemKind::Method(sig, body) => {
let (sig, body) = expand_and_rename_method(sig, body, fld);
ast::ImplItemKind::Method(sig, body)
Expand All @@ -1072,13 +1079,11 @@ fn expand_impl_item(ii: ast::ImplItem, fld: &mut MacroExpander)
},
span: fld.new_span(ii.span)
}),
ast::ImplItemKind::Macro(_) => {
let (span, mac) = match ii.node {
ast::ImplItemKind::Macro(mac) => (ii.span, mac),
_ => unreachable!()
};
ast::ImplItemKind::Macro(mac) => {
check_attributes(&ii.attrs, fld);

let maybe_new_items =
expand_mac_invoc(mac, span,
expand_mac_invoc(mac, ii.span,
|r| r.make_impl_items(),
|meths, mark| meths.move_map(|m| mark_impl_item(m, mark)),
fld);
Expand Down
117 changes: 27 additions & 90 deletions src/libsyntax/feature_gate.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -688,7 +688,7 @@ pub fn check_for_pushpop_syntax(f: Option<&Features>, diag: &Handler, span: Span
}

struct Context<'a> {
features: Features,
features: &'a Features,
span_handler: &'a Handler,
cm: &'a CodeMap,
plugin_attributes: &'a [(String, AttributeType)],
Expand Down Expand Up @@ -739,9 +739,7 @@ impl<'a> Context<'a> {
with the prefix `rustc_` \
are reserved for internal compiler diagnostics");
} else if name.starts_with("derive_") {
gate_feature!(self, custom_derive, attr.span,
"attributes of the form `#[derive_*]` are reserved \
for the compiler");
gate_feature!(self, custom_derive, attr.span, EXPLAIN_DERIVE_UNDERSCORE);
} else {
// Only run the custom attribute lint during regular
// feature gate checking. Macro gating runs
Expand All @@ -759,6 +757,15 @@ impl<'a> Context<'a> {
}
}

pub fn check_attribute(attr: &ast::Attribute, handler: &Handler,
cm: &CodeMap, features: &Features) {
let cx = Context {
features: features, span_handler: handler,
cm: cm, plugin_attributes: &[]
};
cx.check_attribute(attr, true);
}

fn find_lang_feature_issue(feature: &str) -> Option<u32> {
if let Some(info) = ACTIVE_FEATURES.iter().find(|t| t.0 == feature) {
let issue = info.2;
Expand Down Expand Up @@ -819,64 +826,8 @@ pub const EXPLAIN_ALLOW_INTERNAL_UNSTABLE: &'static str =
pub const EXPLAIN_CUSTOM_DERIVE: &'static str =
"`#[derive]` for custom traits is not stable enough for use and is subject to change";

struct MacroVisitor<'a> {
context: &'a Context<'a>
}

impl<'a, 'v> Visitor<'v> for MacroVisitor<'a> {
fn visit_mac(&mut self, mac: &ast::Mac) {
let path = &mac.node.path;
let name = path.segments.last().unwrap().identifier.name.as_str();

// Issue 22234: If you add a new case here, make sure to also
// add code to catch the macro during or after expansion.
//
// We still keep this MacroVisitor (rather than *solely*
// relying on catching cases during or after expansion) to
// catch uses of these macros within conditionally-compiled
// code, e.g. `#[cfg]`-guarded functions.

if name == "asm" {
gate_feature!(self.context, asm, path.span, EXPLAIN_ASM);
}

else if name == "log_syntax" {
gate_feature!(self.context, log_syntax, path.span, EXPLAIN_LOG_SYNTAX);
}

else if name == "trace_macros" {
gate_feature!(self.context, trace_macros, path.span, EXPLAIN_TRACE_MACROS);
}

else if name == "concat_idents" {
gate_feature!(self.context, concat_idents, path.span, EXPLAIN_CONCAT_IDENTS);
}
}

fn visit_attribute(&mut self, attr: &'v ast::Attribute) {
self.context.check_attribute(attr, true);
}

fn visit_expr(&mut self, e: &ast::Expr) {
// Issue 22181: overloaded-`box` and placement-`in` are
// implemented via a desugaring expansion, so their feature
// gates go into MacroVisitor since that works pre-expansion.
//
// Issue 22234: we also check during expansion as well.
// But we keep these checks as a pre-expansion check to catch
// uses in e.g. conditionalized code.

if let ast::ExprKind::Box(_) = e.node {
gate_feature!(self.context, box_syntax, e.span, EXPLAIN_BOX_SYNTAX);
}

if let ast::ExprKind::InPlace(..) = e.node {
gate_feature!(self.context, placement_in_syntax, e.span, EXPLAIN_PLACEMENT_IN);
}

visit::walk_expr(self, e);
}
}
pub const EXPLAIN_DERIVE_UNDERSCORE: &'static str =
"attributes of the form `#[derive_*]` are reserved for the compiler";

struct PostExpansionVisitor<'a> {
context: &'a Context<'a>,
Expand Down Expand Up @@ -1177,13 +1128,7 @@ impl<'a, 'v> Visitor<'v> for PostExpansionVisitor<'a> {
}
}

fn check_crate_inner<F>(cm: &CodeMap, span_handler: &Handler,
krate: &ast::Crate,
plugin_attributes: &[(String, AttributeType)],
check: F)
-> Features
where F: FnOnce(&mut Context, &ast::Crate)
{
pub fn get_features(span_handler: &Handler, krate: &ast::Crate) -> Features {
let mut features = Features::new();

for attr in &krate.attrs {
Expand Down Expand Up @@ -1226,32 +1171,24 @@ fn check_crate_inner<F>(cm: &CodeMap, span_handler: &Handler,
}
}

let mut cx = Context {
features: features,
span_handler: span_handler,
cm: cm,
plugin_attributes: plugin_attributes,
};

check(&mut cx, krate);
cx.features
}

pub fn check_crate_macros(cm: &CodeMap, span_handler: &Handler, krate: &ast::Crate)
-> Features {
check_crate_inner(cm, span_handler, krate, &[] as &'static [_],
|ctx, krate| visit::walk_crate(&mut MacroVisitor { context: ctx }, krate))
features
}

pub fn check_crate(cm: &CodeMap, span_handler: &Handler, krate: &ast::Crate,
plugin_attributes: &[(String, AttributeType)],
unstable: UnstableFeatures) -> Features
{
unstable: UnstableFeatures) -> Features {
maybe_stage_features(span_handler, krate, unstable);

check_crate_inner(cm, span_handler, krate, plugin_attributes,
|ctx, krate| visit::walk_crate(&mut PostExpansionVisitor { context: ctx },
krate))
let features = get_features(span_handler, krate);
{
let ctx = Context {
features: &features,
span_handler: span_handler,
cm: cm,
plugin_attributes: plugin_attributes,
};
visit::walk_crate(&mut PostExpansionVisitor { context: &ctx }, krate);
}
features
}

#[derive(Clone, Copy)]
Expand Down
Loading

0 comments on commit 8b40531

Please sign in to comment.