[Macros/Parser] Attributes on MacroExpansionDecl

* Parse `#<identifier>` attribute list as a `MacroExpansionDecl`
  regardless of the position
* Diagnose whitespaces between `#` and the macro name.
* Correctly attach attributes to `MacroExpansionDecl`
* Fix `OrigDeclAttributes` to handle modifiers (use `getLocation()`
  instead of `AtLoc`.)

Type checking is a TODO

rdar://107386648
(cherry picked from commit 9fc1521)

Author: rintaro

include/swift/Parse/Parser.h

@@ -900,11 +900,15 @@ class Parser {
   // Decl Parsing
 
   /// Returns true if parser is at the start of a Swift decl or decl-import.
-  bool isStartOfSwiftDecl(bool allowPoundIfAttributes = true);
+  bool isStartOfSwiftDecl(bool allowPoundIfAttributes = true,
+                          bool hadAttrsOrModifiers = false);
 
   /// Returns true if the parser is at the start of a SIL decl.
   bool isStartOfSILDecl();
 
+  /// Returns true if the parser is at a freestanding macro expansion.
+  bool isStartOfFreestandingMacroExpansion();
+
   /// Parse the top-level Swift items into the provided vector.
   ///
   /// Each item will be a declaration, statement, or expression.

lib/AST/Attr.cpp

@@ -856,7 +856,7 @@ OrigDeclAttrFilter::operator()(const DeclAttribute *Attr) const {
   auto declLoc = decl->getStartLoc();
   auto *mod = decl->getModuleContext();
   auto *declFile = mod->getSourceFileContainingLocation(declLoc);
-  auto *attrFile = mod->getSourceFileContainingLocation(Attr->AtLoc);
+  auto *attrFile = mod->getSourceFileContainingLocation(Attr->getLocation());
   if (!declFile || !attrFile)
     return Attr;
 

lib/Parse/ParseDecl.cpp

@@ -4768,7 +4768,8 @@ static void skipAttribute(Parser &P) {
   }
 }
 
-bool Parser::isStartOfSwiftDecl(bool allowPoundIfAttributes) {
+bool Parser::isStartOfSwiftDecl(bool allowPoundIfAttributes,
+                                bool hadAttrsOrModifiers) {
   if (Tok.is(tok::at_sign) && peekToken().is(tok::kw_rethrows)) {
     // @rethrows does not follow the general rule of @<identifier> so
     // it is needed to short circuit this else there will be an infinite
@@ -4817,22 +4818,40 @@ bool Parser::isStartOfSwiftDecl(bool allowPoundIfAttributes) {
         (Tok.is(tok::pound_endif) && !allowPoundIfAttributes))
       return true;
 
-    return isStartOfSwiftDecl(allowPoundIfAttributes);
+    return isStartOfSwiftDecl(allowPoundIfAttributes,
+                              /*hadAttrsOrModifiers=*/true);
   }
 
-  if (Tok.is(tok::pound) && peekToken().is(tok::identifier)) {
-    // Macro expansions at the top level are declarations.
-    return !isInSILMode() && SF.Kind != SourceFileKind::Interface &&
-        CurDeclContext->isModuleScopeContext() && !allowTopLevelCode();
+  if (Tok.is(tok::pound)) {
+    if (isStartOfFreestandingMacroExpansion()) {
+      if (isInSILMode() || SF.Kind == SourceFileKind::Interface)
+        return false;
+
+      // Parse '#<identifier>' after attrs/modifiers as a macro expansion decl.
+      if (hadAttrsOrModifiers)
+        return true;
+
+      // Macro expansions at the top level of non-script file are declarations.
+      return CurDeclContext->isModuleScopeContext() && !allowTopLevelCode();
+    }
+
+    // Otherwise, prefer parsing it as an expression.
+    return false;
   }
 
   // Skip a #if that contains only attributes in all branches. These will be
   // parsed as attributes of a declaration, not as separate declarations.
   if (Tok.is(tok::pound_if) && allowPoundIfAttributes) {
     BacktrackingScope backtrack(*this);
     bool sawAnyAttributes = false;
-    return skipIfConfigOfAttributes(sawAnyAttributes) &&
-        (Tok.is(tok::eof) || (sawAnyAttributes && isStartOfSwiftDecl()));
+    if (!skipIfConfigOfAttributes(sawAnyAttributes))
+      return false;
+    if (Tok.is(tok::eof))
+      return true;
+    if (!sawAnyAttributes)
+      return false;
+    return isStartOfSwiftDecl(/*allowPoundIfAttributes=*/true,
+                              /*hadAttrsOrModifiers=*/true);
   }
 
   // If we have a decl modifying keyword, check if the next token is a valid
@@ -4854,13 +4873,15 @@ bool Parser::isStartOfSwiftDecl(bool allowPoundIfAttributes) {
           // If we found the start of a decl while trying to skip over the
           // paren, then we have something incomplete like 'private('. Return
           // true for better recovery.
-          if (isStartOfSwiftDecl(/*allowPoundIfAttributes=*/false))
+          if (isStartOfSwiftDecl(/*allowPoundIfAttributes=*/false,
+                                 /*hadAttrsOrModifiers=*/true))
             return true;
 
           skipSingle();
         }
       }
-      return isStartOfSwiftDecl(/*allowPoundIfAttributes=*/false);
+      return isStartOfSwiftDecl(/*allowPoundIfAttributes=*/false,
+                                /*hadAttrsOrModifiers=*/true);
     }
   }
 
@@ -4887,7 +4908,8 @@ bool Parser::isStartOfSwiftDecl(bool allowPoundIfAttributes) {
     consumeToken(tok::l_paren);
     consumeToken(tok::identifier);
     consumeToken(tok::r_paren);
-    return isStartOfSwiftDecl(/*allowPoundIfAttributes=*/false);
+    return isStartOfSwiftDecl(/*allowPoundIfAttributes=*/false,
+                              /*hadAttrsOrModifiers=*/true);
   }
 
   if (Tok.isContextualKeyword("actor")) {
@@ -4899,7 +4921,8 @@ bool Parser::isStartOfSwiftDecl(bool allowPoundIfAttributes) {
     // it's an actor declaration, otherwise, it isn't.
     do {
       consumeToken();
-    } while (isStartOfSwiftDecl(/*allowPoundIfAttributes=*/false));
+    } while (isStartOfSwiftDecl(/*allowPoundIfAttributes=*/false,
+                                /*hadAttrsOrModifiers=*/true));
     return Tok.is(tok::identifier);
   }
 
@@ -4942,12 +4965,14 @@ bool Parser::isStartOfSwiftDecl(bool allowPoundIfAttributes) {
           // If we found the start of a decl while trying to skip over the
           // paren, then we have something incomplete like 'package('. Return
           // true for better recovery.
-          if (isStartOfSwiftDecl(/*allowPoundIfAttributes=*/false))
+          if (isStartOfSwiftDecl(/*allowPoundIfAttributes=*/false,
+                                 /*hadAttrsOrModifiers=*/true))
             return true;
           skipSingle();
         }
       }
-      return isStartOfSwiftDecl(/*allowPoundIfAttributes=*/false);
+      return isStartOfSwiftDecl(/*allowPoundIfAttributes=*/false,
+                                /*hadAttrsOrModifiers=*/true);
     }
   }
 
@@ -4958,7 +4983,8 @@ bool Parser::isStartOfSwiftDecl(bool allowPoundIfAttributes) {
   // Otherwise, do a recursive parse.
   Parser::BacktrackingScope Backtrack(*this);
   consumeToken(tok::identifier);
-  return isStartOfSwiftDecl(/*allowPoundIfAttributes=*/false);
+  return isStartOfSwiftDecl(/*allowPoundIfAttributes=*/false,
+                            /*hadAttrsOrModifiers=*/true);
 }
 
 bool Parser::isStartOfSILDecl() {
@@ -4986,6 +5012,19 @@ bool Parser::isStartOfSILDecl() {
   llvm_unreachable("Unhandled case in switch");
 }
 
+bool Parser::isStartOfFreestandingMacroExpansion() {
+  // Check if "'#' <identifier>" without any whitespace between them.
+  if (!Tok.is(tok::pound))
+    return false;
+  if (Tok.getRange().getEnd() != peekToken().getLoc())
+    return false;
+  if (!peekToken().isAny(tok::identifier, tok::code_complete) &&
+      // allow keywords right after '#' so we can diagnose it when parsing.
+      !peekToken().isKeyword())
+    return false;
+  return true;
+}
+
 void Parser::consumeDecl(ParserPosition BeginParserPosition,
                          ParseDeclOptions Flags,
                          bool IsTopLevel) {
@@ -5238,9 +5277,15 @@ Parser::parseDecl(ParseDeclOptions Flags,
     // Handled below.
     break;
   case tok::pound:
-    if (Tok.isAtStartOfLine() &&
-        peekToken().is(tok::code_complete) &&
-        Tok.getLoc().getAdvancedLoc(1) == peekToken().getLoc()) {
+    if (!isStartOfFreestandingMacroExpansion()) {
+      consumeToken(tok::pound);
+      diagnose(Tok.getLoc(),
+               diag::macro_expansion_decl_expected_macro_identifier);
+      DeclResult = makeParserError();
+      break;
+    }
+
+    if (peekToken().is(tok::code_complete)) {
       consumeToken();
       if (CodeCompletionCallbacks) {
         CodeCompletionCallbacks->completeAfterPoundDirective();
@@ -5252,6 +5297,7 @@ Parser::parseDecl(ParseDeclOptions Flags,
 
     // Parse as a macro expansion.
     DeclResult = parseDeclMacroExpansion(Flags, Attributes);
+    StaticLoc = SourceLoc(); // Ignore 'static' on macro expansion
     break;
 
   case tok::pound_if:
@@ -9785,10 +9831,10 @@ Parser::parseDeclMacroExpansion(ParseDeclOptions flags,
     }
   }
 
-  return makeParserResult(
-      status,
-      new (Context) MacroExpansionDecl(
-        CurDeclContext, poundLoc, macroNameRef, macroNameLoc,
-        leftAngleLoc, Context.AllocateCopy(genericArgs), rightAngleLoc,
-        argList));
+  auto *med = new (Context) MacroExpansionDecl(
+      CurDeclContext, poundLoc, macroNameRef, macroNameLoc, leftAngleLoc,
+      Context.AllocateCopy(genericArgs), rightAngleLoc, argList);
+  med->getAttrs() = attributes;
+
+  return makeParserResult(status, med);
 }

lib/Parse/ParseExpr.cpp

@@ -1841,6 +1841,12 @@ ParserResult<Expr> Parser::parseExprPrimary(Diag<> ID, bool isExprBasic) {
   }
 
   case tok::pound:
+    if (!isStartOfFreestandingMacroExpansion()) {
+      consumeToken(tok::pound);
+      diagnose(Tok.getLoc(),
+               diag::macro_expansion_expr_expected_macro_identifier);
+      return makeParserError();
+    }
     if (peekToken().is(tok::code_complete) &&
         Tok.getLoc().getAdvancedLoc(1) == peekToken().getLoc()) {
       return parseExprPoundCodeCompletion(/*ParentKind*/None);

lib/Sema/TypeCheckDeclPrimary.cpp

@@ -2058,6 +2058,10 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
   }
 
   void visitMacroExpansionDecl(MacroExpansionDecl *MED) {
+    // TODO: Type check attributes.
+    // Type checking arguments should reflect the attributes.
+    // e.g. '@available(macOS 999) #Future { newAPIFrom999() }'.
+
     // Assign a discriminator.
     (void)MED->getDiscriminator();
     // Decls in expansion already visited as auxiliary decls.

test/Parse/line-directive.swift

@@ -30,8 +30,8 @@ public struct S {
 // expected-error@+5{{operators must have one or two arguments}}
 // expected-error@+4{{member operator '/()' must have at least one argument of type 'S'}}
 // expected-error@+3{{expected '{' in body of function declaration}}
-// expected-error@+2 2 {{consecutive declarations on a line must be separated by ';}}
-// expected-error@+1 2 {{expected a macro identifier}}
+// expected-error@+2 {{consecutive declarations on a line must be separated by ';}}
+// expected-error@+1 {{expected a macro identifier}}
 / ###line 25 "line-directive.swift"
 }
 // expected-error@+1{{#line directive was renamed to #sourceLocation}}

test/Parse/macro_decl.swift

@@ -16,6 +16,11 @@ extension String {
 
   #memberwiseInit(flavor: .chocolate, haha: true) { "abc" }
 
+  @available(macOS 999, *)
+  public #memberwiseInit()
+
+  static internal #memberwiseInit
+
   struct Foo {
     #memberwiseInit
 
@@ -26,3 +31,18 @@ extension String {
   // expected-error @+1 {{expected a macro identifier for a pound literal declaration}}
   #()
 }
+
+@RandomAttr #someFunc
+
+public #someFunc
+
+#someFunc
+
+func test() {
+  @discardableResult #someFunc
+
+  dynamic #someFunc
+
+  @CustomAttr
+  isolated #someFunc
+}

test/Parse/object_literals.swift

@@ -4,5 +4,4 @@ let _ = #notAPound // expected-error {{no macro named 'notAPound'}}
 let _ = #notAPound(1, 2) // expected-error {{no macro named 'notAPound'}}
 let _ = #Color // expected-error {{no macro named 'Color'}}
 
-let _ = [##] // expected-error 2 {{expected a macro identifier}} {{none}}
-// expected-error @-1 {{consecutive statements on a line must be separated by ';'}}
+let _ = [##] // expected-error {{expected a macro identifier}} {{none}}

test/Parse/operator_decl.swift

@@ -52,7 +52,6 @@ infix operator aa--: A // expected-error {{'aa' is considered an identifier and
 infix operator <<$$@< // expected-error {{'$$' is considered an identifier and must not appear within an operator name}}
 infix operator !!@aa // expected-error {{'@' is not allowed in operator names}}
 infix operator #++= // expected-error {{'#' is not allowed in operator names}}
-// expected-error@-1 {{expected a macro identifier}}
 infix operator ++=# // expected-error {{'#' is not allowed in operator names}}
 infix operator -># // expected-error {{'#' is not allowed in operator names}}
 
@@ -61,7 +60,6 @@ infix operator -># // expected-error {{'#' is not allowed in operator names}}
 infix operator =#=
 // expected-error@-1 {{'#' is not allowed in operator names}}
 // expected-error@-2 {{'=' must have consistent whitespace on both sides}}
-// expected-error@-3 {{expected a macro identifier}}
 
 infix operator +++=
 infix operator *** : A

test/Parse/raw_string_errors.swift

@@ -12,15 +12,15 @@ let _ = #"\##("invalid")"#
 let _ = ###"""invalid"######
 // expected-error@-1{{too many '#' characters in closing delimiter}}{{26-29=}}
 // expected-error@-2{{consecutive statements on a line must be separated by ';'}}
-// expected-error@-3 3 {{expected a macro identifier}}
+// expected-error@-3{{expected a macro identifier}}
 
 let _ = ####"invalid"###
 // expected-error@-1{{unterminated string literal}}
 
 let _ = ###"invalid"######
 // expected-error@-1{{too many '#' characters in closing delimiter}}{{24-27=}}
 // expected-error@-2{{consecutive statements on a line must be separated by ';'}}
-// expected-error@-3 3 {{expected a macro identifier}}
+// expected-error@-3{{expected a macro identifier}}
 
 let _ = ##"""aa
   foobar