Syntactically permit attributes on 'if' expressions

src/librustc_ast_passes/ast_validation.rs

@@ -515,6 +515,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
                 )
                 .emit();
             }
+            ExprKind::If(..) => attr::check_attr_on_if_expr(&*expr.attrs, self.err_handler()),
             _ => {}
         }
 

src/librustc_parse/config.rs

@@ -251,6 +251,7 @@ const CFG_ATTR_NOTE_REF: &str = "for more information, visit \
 
 impl<'a> StripUnconfigured<'a> {
     pub fn configure<T: HasAttrs>(&mut self, mut node: T) -> Option<T> {
+        node.check_cfg_attrs(&self.sess.span_diagnostic);
         self.process_cfg_attrs(&mut node);
         self.in_cfg(node.attrs()).then_some(node)
     }

src/librustc_parse/parser/expr.rs

@@ -676,20 +676,11 @@ impl<'a> Parser<'a> {
             expr.map(|mut expr| {
                 attrs.extend::<Vec<_>>(expr.attrs.into());
                 expr.attrs = attrs;
-                self.error_attr_on_if_expr(&expr);
                 expr
             })
         })
     }
 
-    fn error_attr_on_if_expr(&self, expr: &Expr) {
-        if let (ExprKind::If(..), [a0, ..]) = (&expr.kind, &*expr.attrs) {
-            // Just point to the first attribute in there...
-            self.struct_span_err(a0.span, "attributes are not yet allowed on `if` expressions")
-                .emit();
-        }
-    }
-
     fn parse_dot_or_call_expr_with_(&mut self, mut e: P<Expr>, lo: Span) -> PResult<'a, P<Expr>> {
         loop {
             if self.eat(&token::Question) {

src/libsyntax/attr/mod.rs

@@ -10,14 +10,15 @@ pub use StabilityLevel::*;
 
 use crate::ast;
 use crate::ast::{AttrId, AttrItem, AttrKind, AttrStyle, AttrVec, Ident, Name, Path, PathSegment};
-use crate::ast::{Expr, GenericParam, Item, Lit, LitKind, Local, Stmt, StmtKind};
+use crate::ast::{Expr, ExprKind, GenericParam, Item, Lit, LitKind, Local, Stmt, StmtKind};
 use crate::ast::{MacArgs, MacDelimiter, MetaItem, MetaItemKind, NestedMetaItem};
 use crate::mut_visit::visit_clobber;
 use crate::ptr::P;
 use crate::token::{self, Token};
 use crate::tokenstream::{DelimSpan, TokenStream, TokenTree, TreeAndJoint};
 use crate::GLOBALS;
 
+use rustc_errors::Handler;
 use rustc_span::source_map::{BytePos, Spanned};
 use rustc_span::symbol::{sym, Symbol};
 use rustc_span::Span;
@@ -26,6 +27,22 @@ use log::debug;
 use std::iter;
 use std::ops::DerefMut;
 
+/// The central location for denying the precense of attributes on an 'if' expression.
+/// If attributes on 'if' expres are ever permitted, this is the place to add
+/// the feature-gate check.
+pub fn check_attr_on_if_expr(attrs: &[Attribute], err_handler: &Handler) {
+    if let [a0, ..] = attrs {
+        // Deduplicate errors by attr id, as this method may get multiple times
+        // for the same set of attrs
+        if GLOBALS.with(|globals| globals.if_expr_attrs.lock().insert(a0.id)) {
+            // Just point to the first attribute in there...
+            err_handler
+                .struct_span_err(a0.span, "attributes are not yet allowed on `if` expressions")
+                .emit();
+        }
+    }
+}
+
 pub fn mark_used(attr: &Attribute) {
     debug!("marking {:?} as used", attr);
     GLOBALS.with(|globals| {
@@ -626,11 +643,19 @@ impl NestedMetaItem {
 }
 
 pub trait HasAttrs: Sized {
+    /// A hook invoked before any `#[cfg]` or `#[cfg_attr]`
+    /// attributes are processed. Override this if you want
+    /// to prevent `#[cfg]` or `#[cfg_attr]` from being used
+    /// on something (e.g. an `Expr`)
+    fn check_cfg_attrs(&self, _handler: &Handler) {}
     fn attrs(&self) -> &[ast::Attribute];
     fn visit_attrs<F: FnOnce(&mut Vec<ast::Attribute>)>(&mut self, f: F);
 }
 
 impl<T: HasAttrs> HasAttrs for Spanned<T> {
+    fn check_cfg_attrs(&self, handler: &Handler) {
+        self.node.check_cfg_attrs(handler)
+    }
     fn attrs(&self) -> &[ast::Attribute] {
         self.node.attrs()
     }
@@ -662,6 +687,9 @@ impl HasAttrs for AttrVec {
 }
 
 impl<T: HasAttrs + 'static> HasAttrs for P<T> {
+    fn check_cfg_attrs(&self, handler: &Handler) {
+        (**self).check_cfg_attrs(handler);
+    }
     fn attrs(&self) -> &[Attribute] {
         (**self).attrs()
     }
@@ -671,6 +699,16 @@ impl<T: HasAttrs + 'static> HasAttrs for P<T> {
 }
 
 impl HasAttrs for StmtKind {
+    fn check_cfg_attrs(&self, handler: &Handler) {
+        debug!("check_cfg_attrs: StmtKind {:?}", self);
+        match *self {
+            StmtKind::Expr(ref expr) | StmtKind::Semi(ref expr) => {
+                expr.check_cfg_attrs(handler);
+            }
+            _ => {}
+        }
+    }
+
     fn attrs(&self) -> &[Attribute] {
         match *self {
             StmtKind::Local(ref local) => local.attrs(),
@@ -698,6 +736,9 @@ impl HasAttrs for StmtKind {
 }
 
 impl HasAttrs for Stmt {
+    fn check_cfg_attrs(&self, handler: &Handler) {
+        self.kind.check_cfg_attrs(handler)
+    }
     fn attrs(&self) -> &[ast::Attribute] {
         self.kind.attrs()
     }
@@ -717,6 +758,22 @@ impl HasAttrs for GenericParam {
     }
 }
 
+impl HasAttrs for Expr {
+    fn check_cfg_attrs(&self, handler: &Handler) {
+        debug!("check_cfg_attrs: Expr {:?}", self);
+        if let ExprKind::If(..) = &self.kind {
+            check_attr_on_if_expr(&self.attrs, handler);
+        }
+    }
+    fn attrs(&self) -> &[Attribute] {
+        &self.attrs
+    }
+
+    fn visit_attrs<F: FnOnce(&mut Vec<Attribute>)>(&mut self, f: F) {
+        self.attrs.visit_attrs(f);
+    }
+}
+
 macro_rules! derive_has_attrs {
     ($($ty:path),*) => { $(
         impl HasAttrs for $ty {
@@ -732,6 +789,6 @@ macro_rules! derive_has_attrs {
 }
 
 derive_has_attrs! {
-    Item, Expr, Local, ast::ForeignItem, ast::StructField, ast::AssocItem, ast::Arm,
+    Item, Local, ast::ForeignItem, ast::StructField, ast::AssocItem, ast::Arm,
     ast::Field, ast::FieldPat, ast::Variant, ast::Param
 }

src/libsyntax/lib.rs

@@ -35,6 +35,10 @@ macro_rules! unwrap_or {
 pub struct Globals {
     used_attrs: Lock<GrowableBitSet<AttrId>>,
     known_attrs: Lock<GrowableBitSet<AttrId>>,
+    /// Stores attributes on 'if' expressions that
+    /// we've already emitted an error for, to avoid emitting
+    /// duplicate errors
+    if_expr_attrs: Lock<GrowableBitSet<AttrId>>,
     rustc_span_globals: rustc_span::Globals,
 }
 
@@ -45,6 +49,7 @@ impl Globals {
             // initiate the vectors with 0 bits. We'll grow them as necessary.
             used_attrs: Lock::new(GrowableBitSet::new_empty()),
             known_attrs: Lock::new(GrowableBitSet::new_empty()),
+            if_expr_attrs: Lock::new(GrowableBitSet::new_empty()),
             rustc_span_globals: rustc_span::Globals::new(edition),
         }
     }

src/test/pretty/if-attr.rs

@@ -0,0 +1,28 @@
+// pp-exact
+
+#[cfg(FALSE)]
+fn simple_attr() {
+
+    #[attr]
+    if true { }
+
+    #[allow_warnings]
+    if true { }
+}
+
+#[cfg(FALSE)]
+fn if_else_chain() {
+
+    #[first_attr]
+    if true { } else if false { } else { }
+}
+
+#[cfg(FALSE)]
+fn if_let() {
+
+    #[attr]
+    if let Some(_) = Some(true) { }
+}
+
+
+fn main() { }

src/test/ui/parser/attr-stmt-expr-attr-bad.rs

@@ -38,8 +38,6 @@ fn main() {}
 //~^ ERROR an inner attribute is not permitted in this context
 #[cfg(FALSE)] fn e() { let _ = #[attr] &mut #![attr] 0; }
 //~^ ERROR an inner attribute is not permitted in this context
-#[cfg(FALSE)] fn e() { let _ = #[attr] if 0 {}; }
-//~^ ERROR attributes are not yet allowed on `if` expressions
 #[cfg(FALSE)] fn e() { let _ = if 0 #[attr] {}; }
 //~^ ERROR expected `{`, found `#`
 #[cfg(FALSE)] fn e() { let _ = if 0 {#![attr]}; }
@@ -51,14 +49,11 @@ fn main() {}
 #[cfg(FALSE)] fn e() { let _ = if 0 {} else {#![attr]}; }
 //~^ ERROR an inner attribute is not permitted in this context
 #[cfg(FALSE)] fn e() { let _ = if 0 {} else #[attr] if 0 {}; }
-//~^ ERROR attributes are not yet allowed on `if` expressions
-//~| ERROR expected `{`, found `#`
+//~^ ERROR expected `{`, found `#`
 #[cfg(FALSE)] fn e() { let _ = if 0 {} else if 0 #[attr] {}; }
 //~^ ERROR expected `{`, found `#`
 #[cfg(FALSE)] fn e() { let _ = if 0 {} else if 0 {#![attr]}; }
 //~^ ERROR an inner attribute is not permitted in this context
-#[cfg(FALSE)] fn e() { let _ = #[attr] if let _ = 0 {}; }
-//~^ ERROR attributes are not yet allowed on `if` expressions
 #[cfg(FALSE)] fn e() { let _ = if let _ = 0 #[attr] {}; }
 //~^ ERROR expected `{`, found `#`
 #[cfg(FALSE)] fn e() { let _ = if let _ = 0 {#![attr]}; }
@@ -70,8 +65,7 @@ fn main() {}
 #[cfg(FALSE)] fn e() { let _ = if let _ = 0 {} else {#![attr]}; }
 //~^ ERROR an inner attribute is not permitted in this context
 #[cfg(FALSE)] fn e() { let _ = if let _ = 0 {} else #[attr] if let _ = 0 {}; }
-//~^ ERROR attributes are not yet allowed on `if` expressions
-//~| ERROR expected `{`, found `#`
+//~^ ERROR expected `{`, found `#`
 #[cfg(FALSE)] fn e() { let _ = if let _ = 0 {} else if let _ = 0 #[attr] {}; }
 //~^ ERROR expected `{`, found `#`
 #[cfg(FALSE)] fn e() { let _ = if let _ = 0 {} else if let _ = 0 {#![attr]}; }