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

Expand Attributes on Statements and Expressions #49124

Merged
merged 1 commit into from
Apr 2, 2018
Merged
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
7 changes: 7 additions & 0 deletions src/libsyntax/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -837,6 +837,13 @@ impl Stmt {
_ => false,
}
}

pub fn is_expr(&self) -> bool {
match self.node {
StmtKind::Expr(_) => true,
_ => false,
}
}
}

impl fmt::Debug for Stmt {
Expand Down
27 changes: 17 additions & 10 deletions src/libsyntax/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,17 +149,24 @@ impl<'a> StripUnconfigured<'a> {
fn visit_expr_attrs(&mut self, attrs: &[ast::Attribute]) {
// flag the offending attributes
for attr in attrs.iter() {
if !self.features.map(|features| features.stmt_expr_attributes).unwrap_or(true) {
let mut err = feature_err(self.sess,
"stmt_expr_attributes",
attr.span,
GateIssue::Language,
EXPLAIN_STMT_ATTR_SYNTAX);
if attr.is_sugared_doc {
err.help("`///` is for documentation comments. For a plain comment, use `//`.");
}
err.emit();
self.maybe_emit_expr_attr_err(attr);
}
}

/// If attributes are not allowed on expressions, emit an error for `attr`
pub fn maybe_emit_expr_attr_err(&self, attr: &ast::Attribute) {
if !self.features.map(|features| features.stmt_expr_attributes).unwrap_or(true) {
let mut err = feature_err(self.sess,
"stmt_expr_attributes",
attr.span,
GateIssue::Language,
EXPLAIN_STMT_ATTR_SYNTAX);

if attr.is_sugared_doc {
err.help("`///` is for documentation comments. For a plain comment, use `//`.");
}

err.emit();
}
}

Expand Down
8 changes: 8 additions & 0 deletions src/libsyntax/ext/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ pub enum Annotatable {
Item(P<ast::Item>),
TraitItem(P<ast::TraitItem>),
ImplItem(P<ast::ImplItem>),
Stmt(P<ast::Stmt>),
Expr(P<ast::Expr>),
}

impl HasAttrs for Annotatable {
Expand All @@ -46,6 +48,8 @@ impl HasAttrs for Annotatable {
Annotatable::Item(ref item) => &item.attrs,
Annotatable::TraitItem(ref trait_item) => &trait_item.attrs,
Annotatable::ImplItem(ref impl_item) => &impl_item.attrs,
Annotatable::Stmt(ref stmt) => stmt.attrs(),
Annotatable::Expr(ref expr) => &expr.attrs,
}
}

Expand All @@ -54,6 +58,8 @@ impl HasAttrs for Annotatable {
Annotatable::Item(item) => Annotatable::Item(item.map_attrs(f)),
Annotatable::TraitItem(trait_item) => Annotatable::TraitItem(trait_item.map_attrs(f)),
Annotatable::ImplItem(impl_item) => Annotatable::ImplItem(impl_item.map_attrs(f)),
Annotatable::Stmt(stmt) => Annotatable::Stmt(stmt.map_attrs(f)),
Annotatable::Expr(expr) => Annotatable::Expr(expr.map_attrs(f)),
}
}
}
Expand All @@ -64,6 +70,8 @@ impl Annotatable {
Annotatable::Item(ref item) => item.span,
Annotatable::TraitItem(ref trait_item) => trait_item.span,
Annotatable::ImplItem(ref impl_item) => impl_item.span,
Annotatable::Stmt(ref stmt) => stmt.span,
Annotatable::Expr(ref expr) => expr.span,
}
}

Expand Down
89 changes: 70 additions & 19 deletions src/libsyntax/ext/expand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -435,6 +435,12 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
Annotatable::ImplItem(item) => {
Annotatable::ImplItem(item.map(|item| cfg.fold_impl_item(item).pop().unwrap()))
}
Annotatable::Stmt(stmt) => {
Annotatable::Stmt(stmt.map(|stmt| cfg.fold_stmt(stmt).pop().unwrap()))
}
Annotatable::Expr(expr) => {
Annotatable::Expr(cfg.fold_expr(expr))
}
}
}

Expand Down Expand Up @@ -503,6 +509,8 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
Annotatable::Item(item) => token::NtItem(item),
Annotatable::TraitItem(item) => token::NtTraitItem(item.into_inner()),
Annotatable::ImplItem(item) => token::NtImplItem(item.into_inner()),
Annotatable::Stmt(stmt) => token::NtStmt(stmt.into_inner()),
Annotatable::Expr(expr) => token::NtExpr(expr),
})).into();
let tok_result = mac.expand(self.cx, attr.span, attr.tokens, item_tok);
self.parse_expansion(tok_result, kind, &attr.path, attr.span)
Expand Down Expand Up @@ -751,6 +759,7 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
Some(expansion)
}
Err(mut err) => {
err.set_span(span);
err.emit();
self.cx.trace_macros_diag();
kind.dummy(span)
Expand Down Expand Up @@ -796,7 +805,13 @@ impl<'a> Parser<'a> {
Expansion::Stmts(stmts)
}
ExpansionKind::Expr => Expansion::Expr(self.parse_expr()?),
ExpansionKind::OptExpr => Expansion::OptExpr(Some(self.parse_expr()?)),
ExpansionKind::OptExpr => {
if self.token != token::Eof {
Expansion::OptExpr(Some(self.parse_expr()?))
} else {
Expansion::OptExpr(None)
}
},
ExpansionKind::Ty => Expansion::Ty(self.parse_ty()?),
ExpansionKind::Pat => Expansion::Pat(self.parse_pat()?),
})
Expand Down Expand Up @@ -904,6 +919,18 @@ impl<'a, 'b> Folder for InvocationCollector<'a, 'b> {
let mut expr = self.cfg.configure_expr(expr).into_inner();
expr.node = self.cfg.configure_expr_kind(expr.node);

let (attr, derives, expr) = self.classify_item(expr);

if attr.is_some() || !derives.is_empty() {
// collect the invoc regardless of whether or not attributes are permitted here
// expansion will eat the attribute so it won't error later
attr.as_ref().map(|a| self.cfg.maybe_emit_expr_attr_err(a));

// ExpansionKind::Expr requires the macro to emit an expression
return self.collect_attr(attr, derives, Annotatable::Expr(P(expr)), ExpansionKind::Expr)
.make_expr();
}

if let ast::ExprKind::Mac(mac) = expr.node {
self.check_attributes(&expr.attrs);
self.collect_bang(mac, expr.span, ExpansionKind::Expr).make_expr()
Expand All @@ -916,6 +943,16 @@ impl<'a, 'b> Folder for InvocationCollector<'a, 'b> {
let mut expr = configure!(self, expr).into_inner();
expr.node = self.cfg.configure_expr_kind(expr.node);

let (attr, derives, expr) = self.classify_item(expr);

if attr.is_some() || !derives.is_empty() {
attr.as_ref().map(|a| self.cfg.maybe_emit_expr_attr_err(a));

return self.collect_attr(attr, derives, Annotatable::Expr(P(expr)),
ExpansionKind::OptExpr)
.make_opt_expr();
}

if let ast::ExprKind::Mac(mac) = expr.node {
self.check_attributes(&expr.attrs);
self.collect_bang(mac, expr.span, ExpansionKind::OptExpr).make_opt_expr()
Expand All @@ -938,33 +975,47 @@ impl<'a, 'b> Folder for InvocationCollector<'a, 'b> {
}

fn fold_stmt(&mut self, stmt: ast::Stmt) -> SmallVector<ast::Stmt> {
let stmt = match self.cfg.configure_stmt(stmt) {
let mut stmt = match self.cfg.configure_stmt(stmt) {
Some(stmt) => stmt,
None => return SmallVector::new(),
};

let (mac, style, attrs) = if let StmtKind::Mac(mac) = stmt.node {
mac.into_inner()
} else {
// The placeholder expander gives ids to statements, so we avoid folding the id here.
let ast::Stmt { id, node, span } = stmt;
return noop_fold_stmt_kind(node, self).into_iter().map(|node| {
ast::Stmt { id: id, node: node, span: span }
}).collect()
};
// we'll expand attributes on expressions separately
if !stmt.is_expr() {
let (attr, derives, stmt_) = self.classify_item(stmt);

if attr.is_some() || !derives.is_empty() {
return self.collect_attr(attr, derives,
Annotatable::Stmt(P(stmt_)), ExpansionKind::Stmts)
.make_stmts();
}

self.check_attributes(&attrs);
let mut placeholder = self.collect_bang(mac, stmt.span, ExpansionKind::Stmts).make_stmts();
stmt = stmt_;
}

// If this is a macro invocation with a semicolon, then apply that
// semicolon to the final statement produced by expansion.
if style == MacStmtStyle::Semicolon {
if let Some(stmt) = placeholder.pop() {
placeholder.push(stmt.add_trailing_semicolon());
if let StmtKind::Mac(mac) = stmt.node {
let (mac, style, attrs) = mac.into_inner();
self.check_attributes(&attrs);
let mut placeholder = self.collect_bang(mac, stmt.span, ExpansionKind::Stmts)
.make_stmts();

// If this is a macro invocation with a semicolon, then apply that
// semicolon to the final statement produced by expansion.
if style == MacStmtStyle::Semicolon {
if let Some(stmt) = placeholder.pop() {
placeholder.push(stmt.add_trailing_semicolon());
}
}

return placeholder;
}

placeholder
// The placeholder expander gives ids to statements, so we avoid folding the id here.
let ast::Stmt { id, node, span } = stmt;
noop_fold_stmt_kind(node, self).into_iter().map(|node| {
ast::Stmt { id, node, span }
}).collect()

}

fn fold_block(&mut self, block: P<Block>) -> P<Block> {
Expand Down
2 changes: 1 addition & 1 deletion src/libsyntax/feature_gate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1250,7 +1250,7 @@ const EXPLAIN_BOX_SYNTAX: &'static str =
"box expression syntax is experimental; you can call `Box::new` instead.";

pub const EXPLAIN_STMT_ATTR_SYNTAX: &'static str =
"attributes on non-item statements and expressions are experimental.";
"attributes on expressions are experimental.";

pub const EXPLAIN_ASM: &'static str =
"inline assembly is not stable enough for use and is subject to change";
Expand Down
3 changes: 3 additions & 0 deletions src/libsyntax/parse/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4601,6 +4601,9 @@ impl<'a> Parser<'a> {

/// Parse a statement, including the trailing semicolon.
pub fn parse_full_stmt(&mut self, macro_legacy_warnings: bool) -> PResult<'a, Option<Stmt>> {
// skip looking for a trailing semicolon when we have an interpolated statement
maybe_whole!(self, NtStmt, |x| Some(x));

let mut stmt = match self.parse_stmt_without_recovery(macro_legacy_warnings)? {
Some(stmt) => stmt,
None => return Ok(None),
Expand Down
4 changes: 3 additions & 1 deletion src/libsyntax_ext/deriving/custom.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,9 @@ impl MultiItemModifier for ProcMacroDerive {
let item = match item {
Annotatable::Item(item) => item,
Annotatable::ImplItem(_) |
Annotatable::TraitItem(_) => {
Annotatable::TraitItem(_) |
Annotatable::Stmt(_) |
Annotatable::Expr(_) => {
ecx.span_err(span, "proc-macro derives may only be \
applied to struct/enum items");
return Vec::new()
Expand Down
4 changes: 4 additions & 0 deletions src/test/compile-fail-fulldeps/auxiliary/macro_crate_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,8 @@ fn expand_into_foo_multi(cx: &mut ExtCtxt,
}
})
}
// these are covered in proc_macro/attr-stmt-expr.rs
Annotatable::Stmt(_) | Annotatable::Expr(_) => panic!("expected item")
}
}

Expand Down Expand Up @@ -145,6 +147,8 @@ fn expand_duplicate(cx: &mut ExtCtxt,
new_it.ident = copy_name;
push(Annotatable::TraitItem(P(new_it)));
}
// covered in proc_macro/attr-stmt-expr.rs
Annotatable::Stmt(_) | Annotatable::Expr(_) => panic!("expected item")
}
}

Expand Down
38 changes: 38 additions & 0 deletions src/test/compile-fail-fulldeps/proc-macro/attr-invalid-exprs.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// aux-build:attr-stmt-expr.rs
// ignore-stage1

//! Attributes producing expressions in invalid locations

#![feature(proc_macro, stmt_expr_attributes)]

extern crate attr_stmt_expr;
use attr_stmt_expr::{duplicate, no_output};

fn main() {
let _ = #[no_output] "Hello, world!";
//~^ ERROR expected expression, found `<eof>`

let _ = #[duplicate] "Hello, world!";
//~^ ERROR macro expansion ignores token `,` and any following

let _ = {
#[no_output]
"Hello, world!"
Copy link
Contributor

Choose a reason for hiding this comment

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

So, you can cfg-out a trailing expression and implicit () will be used instead? Fun.
At least this is consistent with existing behavior of #[cfg].

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that's just the behavior of an empty block, no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

It's behavior of an empty block, yes, but it's not obvious that { #[cfg_like] trailing } should expand into an empty block - it depends on interpretation.
If the attribute is applied to the statement, then 1 statement -> 0 statements transformation is okay, but if the attribute is applied to the expression inside of that statement, then it supposedly should be an error, because you can't shrink an expression in that position into nothing (it's not a part of expression list).

As I understand the code in expand.rs, attributes on expression statements are treated as expression attributes and not statement attributes, but the behavior follows the statement attribute interpretation.
Ok, now I'm actually not sure whether #[no_output] is applied at all here.
We need to add a test making sure that this doesn't expand into let _ = { "Hello, world!" }; ignoring the attribute.

};

let _ = {
#[duplicate]
//~^ ERROR macro expansion ignores token `,` and any following
"Hello, world!"
};
}
38 changes: 38 additions & 0 deletions src/test/compile-fail-fulldeps/proc-macro/attr-stmt-expr.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// aux-build:attr-stmt-expr.rs
// ignore-stage1

#![feature(proc_macro)]

extern crate attr_stmt_expr;
use attr_stmt_expr::{expect_let, expect_print_stmt, expect_expr, expect_print_expr};

fn print_str(string: &'static str) {
// macros are handled a bit differently
#[expect_print_expr]
//~^ ERROR attributes on expressions are experimental
//~| HELP add #![feature(stmt_expr_attributes)] to the crate attributes to enable
println!("{}", string)
}

fn main() {
#[expect_let]
let string = "Hello, world!";

#[expect_print_stmt]
println!("{}", string);

#[expect_expr]
//~^ ERROR attributes on expressions are experimental
//~| HELP add #![feature(stmt_expr_attributes)] to the crate attributes to enable
print_str("string")
}
Loading