Skip to content

Commit

Permalink
Auto merge of #60669 - c410-f3r:attrs-fn, r=petrochenkov
Browse files Browse the repository at this point in the history
Allow attributes in formal function parameters

Implements #60406.

This is my first contribution to the compiler and since this is a large and complex project, I am not fully aware of the consequences of the changes I have made.

**TODO**

- [x] Forbid some built-in attributes.
- [x] Expand cfg/cfg_attr
  • Loading branch information
bors committed Jun 12, 2019
2 parents c4797fa + 1eaaf44 commit 3f511ad
Show file tree
Hide file tree
Showing 28 changed files with 1,115 additions and 74 deletions.
6 changes: 6 additions & 0 deletions src/librustc_data_structures/thin_vec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,3 +66,9 @@ impl<T: HashStable<CTX>, CTX> HashStable<CTX> for ThinVec<T> {
(**self).hash_stable(hcx, hasher)
}
}

impl<T> Default for ThinVec<T> {
fn default() -> Self {
Self(None)
}
}
45 changes: 43 additions & 2 deletions src/librustc_passes/ast_validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ use rustc::session::Session;
use rustc_data_structures::fx::FxHashMap;
use syntax::ast::*;
use syntax::attr;
use syntax::feature_gate::is_builtin_attr;
use syntax::source_map::Spanned;
use syntax::symbol::{kw, sym};
use syntax::ptr::P;
Expand Down Expand Up @@ -365,6 +366,29 @@ impl<'a> AstValidator<'a> {
_ => None,
}
}

fn check_fn_decl(&self, fn_decl: &FnDecl) {
fn_decl
.inputs
.iter()
.flat_map(|i| i.attrs.as_ref())
.filter(|attr| {
let arr = [sym::allow, sym::cfg, sym::cfg_attr, sym::deny, sym::forbid, sym::warn];
!arr.contains(&attr.name_or_empty()) && is_builtin_attr(attr)
})
.for_each(|attr| if attr.is_sugared_doc {
let mut err = self.err_handler().struct_span_err(
attr.span,
"documentation comments cannot be applied to function parameters"
);
err.span_label(attr.span, "doc comments are not allowed here");
err.emit();
}
else {
self.err_handler().span_err(attr.span, "allow, cfg, cfg_attr, deny, \
forbid, and warn are the only allowed built-in attributes in function parameters")
});
}
}

enum GenericPosition {
Expand Down Expand Up @@ -470,6 +494,9 @@ fn validate_generics_order<'a>(
impl<'a> Visitor<'a> for AstValidator<'a> {
fn visit_expr(&mut self, expr: &'a Expr) {
match expr.node {
ExprKind::Closure(_, _, _, ref fn_decl, _, _) => {
self.check_fn_decl(fn_decl);
}
ExprKind::IfLet(_, ref expr, _, _) | ExprKind::WhileLet(_, ref expr, _, _) =>
self.while_if_let_ambiguity(&expr),
ExprKind::InlineAsm(..) if !self.session.target.target.options.allow_asm => {
Expand All @@ -484,6 +511,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
fn visit_ty(&mut self, ty: &'a Ty) {
match ty.node {
TyKind::BareFn(ref bfty) => {
self.check_fn_decl(&bfty.decl);
self.check_decl_no_pat(&bfty.decl, |span, _| {
struct_span_err!(self.session, span, E0561,
"patterns aren't allowed in function pointer types").emit();
Expand Down Expand Up @@ -601,10 +629,11 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
.note("only trait implementations may be annotated with default").emit();
}
}
ItemKind::Fn(_, ref header, ref generics, _) => {
ItemKind::Fn(ref decl, ref header, ref generics, _) => {
self.visit_fn_header(header);
self.check_fn_decl(decl);
// We currently do not permit const generics in `const fn`, as
// this is tantamount to allowing compile-time dependent typing.
self.visit_fn_header(header);
if header.constness.node == Constness::Const {
// Look for const generics and error if we find any.
for param in &generics.params {
Expand Down Expand Up @@ -657,6 +686,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
self.no_questions_in_bounds(bounds, "supertraits", true);
for trait_item in trait_items {
if let TraitItemKind::Method(ref sig, ref block) = trait_item.node {
self.check_fn_decl(&sig.decl);
self.check_trait_fn_not_async(trait_item.span, sig.header.asyncness.node);
self.check_trait_fn_not_const(sig.header.constness);
if block.is_none() {
Expand Down Expand Up @@ -711,6 +741,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
fn visit_foreign_item(&mut self, fi: &'a ForeignItem) {
match fi.node {
ForeignItemKind::Fn(ref decl, _) => {
self.check_fn_decl(decl);
self.check_decl_no_pat(decl, |span, _| {
struct_span_err!(self.session, span, E0130,
"patterns aren't allowed in foreign function declarations")
Expand Down Expand Up @@ -864,6 +895,16 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
"`async fn` is not permitted in the 2015 edition").emit();
}
}

fn visit_impl_item(&mut self, ii: &'a ImplItem) {
match ii.node {
ImplItemKind::Method(ref sig, _) => {
self.check_fn_decl(&sig.decl);
}
_ => {}
}
visit::walk_impl_item(self, ii);
}
}

pub fn check_crate(session: &Session, krate: &Crate) -> (bool, bool) {
Expand Down
4 changes: 3 additions & 1 deletion src/libsyntax/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1770,6 +1770,7 @@ pub struct InlineAsm {
/// E.g., `bar: usize` as in `fn foo(bar: usize)`.
#[derive(Clone, RustcEncodable, RustcDecodable, Debug)]
pub struct Arg {
pub attrs: ThinVec<Attribute>,
pub ty: P<Ty>,
pub pat: P<Pat>,
pub id: NodeId,
Expand Down Expand Up @@ -1817,14 +1818,15 @@ impl Arg {
}
}

pub fn from_self(eself: ExplicitSelf, eself_ident: Ident) -> Arg {
pub fn from_self(attrs: ThinVec<Attribute>, eself: ExplicitSelf, eself_ident: Ident) -> Arg {
let span = eself.span.to(eself_ident.span);
let infer_ty = P(Ty {
id: DUMMY_NODE_ID,
node: TyKind::ImplicitSelf,
span,
});
let arg = |mutbl, ty| Arg {
attrs,
pat: P(Pat {
id: DUMMY_NODE_ID,
node: PatKind::Ident(BindingMode::ByValue(mutbl), eself_ident, None),
Expand Down
2 changes: 1 addition & 1 deletion src/libsyntax/attr/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -723,7 +723,7 @@ macro_rules! derive_has_attrs {

derive_has_attrs! {
Item, Expr, Local, ast::ForeignItem, ast::StructField, ast::ImplItem, ast::TraitItem, ast::Arm,
ast::Field, ast::FieldPat, ast::Variant_
ast::Field, ast::FieldPat, ast::Variant_, ast::Arg
}

pub fn inject(mut krate: ast::Crate, parse_sess: &ParseSess, attrs: &[String]) -> ast::Crate {
Expand Down
9 changes: 9 additions & 0 deletions src/libsyntax/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,10 @@ impl<'a> StripUnconfigured<'a> {
}
}

pub fn configure_fn_decl(&mut self, fn_decl: &mut ast::FnDecl) {
fn_decl.inputs.flat_map_in_place(|arg| self.configure(arg));
}

/// Denies `#[cfg]` on generic parameters until we decide what to do with it.
/// See issue #51279.
pub fn disallow_cfg_on_generic_param(&mut self, param: &ast::GenericParam) {
Expand Down Expand Up @@ -364,6 +368,11 @@ impl<'a> MutVisitor for StripUnconfigured<'a> {
self.configure_pat(pat);
noop_visit_pat(pat, self)
}

fn visit_fn_decl(&mut self, mut fn_decl: &mut P<ast::FnDecl>) {
self.configure_fn_decl(&mut fn_decl);
noop_visit_fn_decl(fn_decl, self);
}
}

fn is_cfg(attr: &ast::Attribute) -> bool {
Expand Down
5 changes: 3 additions & 2 deletions src/libsyntax/ext/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -963,9 +963,10 @@ impl<'a> AstBuilder for ExtCtxt<'a> {
fn arg(&self, span: Span, ident: ast::Ident, ty: P<ast::Ty>) -> ast::Arg {
let arg_pat = self.pat_ident(span, ident);
ast::Arg {
ty,
attrs: ThinVec::default(),
id: ast::DUMMY_NODE_ID,
pat: arg_pat,
id: ast::DUMMY_NODE_ID
ty,
}
}

Expand Down
5 changes: 5 additions & 0 deletions src/libsyntax/ext/expand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1578,6 +1578,11 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> {
*id = self.cx.resolver.next_node_id()
}
}

fn visit_fn_decl(&mut self, mut fn_decl: &mut P<ast::FnDecl>) {
self.cfg.configure_fn_decl(&mut fn_decl);
noop_visit_fn_decl(fn_decl, self);
}
}

pub struct ExpansionConfig<'feat> {
Expand Down
15 changes: 15 additions & 0 deletions src/libsyntax/feature_gate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -557,6 +557,9 @@ declare_features! (
// Allows the user of associated type bounds.
(active, associated_type_bounds, "1.34.0", Some(52662), None),

// Attributes on formal function params
(active, param_attrs, "1.36.0", Some(60406), None),

// Allows calling constructor functions in `const fn`
// FIXME Create issue
(active, const_constructor, "1.37.0", Some(61456), None),
Expand Down Expand Up @@ -2511,6 +2514,18 @@ pub fn check_crate(krate: &ast::Crate,
parse_sess: sess,
plugin_attributes,
};

sess
.param_attr_spans
.borrow()
.iter()
.for_each(|span| gate_feature!(
&ctx,
param_attrs,
*span,
"attributes on function parameters are unstable"
));

let visitor = &mut PostExpansionVisitor {
context: &ctx,
builtin_attributes: &*BUILTIN_ATTRIBUTE_MAP,
Expand Down
3 changes: 2 additions & 1 deletion src/libsyntax/mut_visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -568,8 +568,9 @@ pub fn noop_visit_meta_item<T: MutVisitor>(mi: &mut MetaItem, vis: &mut T) {
vis.visit_span(span);
}

pub fn noop_visit_arg<T: MutVisitor>(Arg { id, pat, ty }: &mut Arg, vis: &mut T) {
pub fn noop_visit_arg<T: MutVisitor>(Arg { attrs, id, pat, ty }: &mut Arg, vis: &mut T) {
vis.visit_id(id);
visit_thin_attrs(attrs, vis);
vis.visit_pat(pat);
vis.visit_ty(ty);
}
Expand Down
11 changes: 10 additions & 1 deletion src/libsyntax/parse/attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,14 @@ const DEFAULT_UNEXPECTED_INNER_ATTR_ERR_MSG: &str = "an inner attribute is not \
permitted in this context";

impl<'a> Parser<'a> {
crate fn parse_arg_attributes(&mut self) -> PResult<'a, Vec<ast::Attribute>> {
let attrs = self.parse_outer_attributes()?;
attrs.iter().for_each(|a|
self.sess.param_attr_spans.borrow_mut().push(a.span)
);
Ok(attrs)
}

/// Parse attributes that appear before an item
crate fn parse_outer_attributes(&mut self) -> PResult<'a, Vec<ast::Attribute>> {
let mut attrs: Vec<ast::Attribute> = Vec::new();
Expand All @@ -35,7 +43,8 @@ impl<'a> Parser<'a> {
};
let inner_parse_policy =
InnerAttributeParsePolicy::NotPermitted { reason: inner_error_reason };
attrs.push(self.parse_attribute_with_inner_parse_policy(inner_parse_policy)?);
let attr = self.parse_attribute_with_inner_parse_policy(inner_parse_policy)?;
attrs.push(attr);
just_parsed_doc_comment = false;
}
token::DocComment(s) => {
Expand Down
8 changes: 4 additions & 4 deletions src/libsyntax/parse/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ crate fn dummy_arg(ident: Ident) -> Arg {
span: ident.span,
id: ast::DUMMY_NODE_ID
};
Arg { ty: P(ty), pat: pat, id: ast::DUMMY_NODE_ID }
Arg { attrs: ThinVec::default(), id: ast::DUMMY_NODE_ID, pat, ty: P(ty) }
}

pub enum Error {
Expand Down Expand Up @@ -1074,11 +1074,11 @@ impl<'a> Parser<'a> {
Err(err)
}

crate fn eat_incorrect_doc_comment(&mut self, applied_to: &str) {
crate fn eat_incorrect_doc_comment_for_arg_type(&mut self) {
if let token::DocComment(_) = self.token.kind {
let mut err = self.diagnostic().struct_span_err(
self.token.span,
&format!("documentation comments cannot be applied to {}", applied_to),
"documentation comments cannot be applied to a function parameter's type",
);
err.span_label(self.token.span, "doc comments are not allowed here");
err.emit();
Expand All @@ -1095,7 +1095,7 @@ impl<'a> Parser<'a> {
self.bump();
let mut err = self.diagnostic().struct_span_err(
sp,
&format!("attributes cannot be applied to {}", applied_to),
"attributes cannot be applied to a function parameter's type",
);
err.span_label(sp, "attributes are not allowed here");
err.emit();
Expand Down
1 change: 1 addition & 0 deletions src/libsyntax/parse/lexer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1490,6 +1490,7 @@ mod tests {
buffered_lints: Lock::new(vec![]),
edition: Edition::from_session(),
ambiguous_block_expr_parse: Lock::new(FxHashMap::default()),
param_attr_spans: Lock::new(Vec::new()),
}
}

Expand Down
2 changes: 2 additions & 0 deletions src/libsyntax/parse/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ pub struct ParseSess {
/// operation token that followed it, but that the parser cannot identify without further
/// analysis.
pub ambiguous_block_expr_parse: Lock<FxHashMap<Span, Span>>,
pub param_attr_spans: Lock<Vec<Span>>
}

impl ParseSess {
Expand All @@ -79,6 +80,7 @@ impl ParseSess {
buffered_lints: Lock::new(vec![]),
edition: Edition::from_session(),
ambiguous_block_expr_parse: Lock::new(FxHashMap::default()),
param_attr_spans: Lock::new(Vec::new()),
}
}

Expand Down
Loading

0 comments on commit 3f511ad

Please sign in to comment.