Merge pull request #13620 from rintaro/parse-braceitems-reorder

[Parse] Small cleanups in parseBraceItems()

Author: rintaro

include/swift/Parse/Parser.h

@@ -710,11 +710,6 @@ class Parser {
   /// Options that control the parsing of declarations.
   typedef OptionSet<ParseDeclFlags> ParseDeclOptions;
 
-  /// Skips the current token if it is '}', and emits a diagnostic.
-  ///
-  /// \returns true if any tokens were skipped.
-  bool skipExtraTopLevelRBraces();
-
   void delayParseFromBeginningToHere(ParserPosition BeginParserPosition,
                                      ParseDeclOptions Flags);
   void consumeDecl(ParserPosition BeginParserPosition, ParseDeclOptions Flags,

lib/Parse/ParseDecl.cpp

@@ -196,8 +196,6 @@ bool Parser::parseTopLevel() {
   // Parse the body of the file.
   SmallVector<ASTNode, 128> Items;
 
-  skipExtraTopLevelRBraces();
-
   // If we are in SIL mode, and if the first token is the start of a sil
   // declaration, parse that one SIL function and return to the top level.  This
   // allows type declarations and other things to be parsed, name bound, and
@@ -280,19 +278,6 @@ bool Parser::parseTopLevel() {
   return FoundTopLevelCodeToExecute;
 }
 
-bool Parser::skipExtraTopLevelRBraces() {
-  if (!Tok.is(tok::r_brace))
-    return false;
-  while (Tok.is(tok::r_brace)) {
-    diagnose(Tok, diag::extra_rbrace)
-        .fixItRemove(Tok.getLoc());
-    consumeToken();
-  }
-  return true;
-}
-
-
-
 static Optional<StringRef>
 getStringLiteralIfNotInterpolated(Parser &P, SourceLoc Loc, const Token &Tok,
                                   StringRef DiagText) {

lib/Parse/ParseStmt.cpp

@@ -245,11 +245,9 @@ ParserStatus Parser::parseBraceItems(SmallVectorImpl<ASTNode> &Entries,
   }
 
   ParserStatus BraceItemsStatus;
-  SmallVector<Decl*, 8> TmpDecls;
 
   bool PreviousHadSemi = true;
-  while ((Kind == BraceItemListKind::TopLevelLibrary ||
-          Tok.isNot(tok::r_brace)) &&
+  while ((IsTopLevel || Tok.isNot(tok::r_brace)) &&
          Tok.isNot(tok::pound_endif) &&
          Tok.isNot(tok::pound_elseif) &&
          Tok.isNot(tok::pound_else) &&
@@ -266,9 +264,13 @@ ParserStatus Parser::parseBraceItems(SmallVectorImpl<ASTNode> &Entries,
 
     SyntaxParsingContext StmtContext(SyntaxContext, SyntaxContextKind::Stmt);
 
-    if (Kind == BraceItemListKind::TopLevelLibrary &&
-        skipExtraTopLevelRBraces())
+    if (Tok.is(tok::r_brace)) {
+      assert(IsTopLevel);
+      diagnose(Tok, diag::extra_rbrace)
+        .fixItRemove(Tok.getLoc());
+      consumeToken();
       continue;
+    }
 
     // Eat invalid tokens instead of allowing them to produce downstream errors.
     if (consumeIf(tok::unknown))
@@ -292,57 +294,29 @@ ParserStatus Parser::parseBraceItems(SmallVectorImpl<ASTNode> &Entries,
 
     // Parse the decl, stmt, or expression.
     PreviousHadSemi = false;
-    if (isStartOfDecl()
-        && Tok.isNot(
-            tok::pound_if, tok::pound_sourceLocation, tok::pound_line)) {
-      ParserResult<Decl> DeclResult = 
-          parseDecl(IsTopLevel ? PD_AllowTopLevel : PD_Default,
-                    [&](Decl *D) {TmpDecls.push_back(D);});
-      if (DeclResult.isParseError()) {
-        NeedParseErrorRecovery = true;
-        if (DeclResult.hasCodeCompletion() && IsTopLevel &&
-            isCodeCompletionFirstPass()) {
-          consumeDecl(BeginParserPosition, None, IsTopLevel);
-          return DeclResult;
-        }
-      }
-      Result = DeclResult.getPtrOrNull();
-
-      for (Decl *D : TmpDecls)
-        Entries.push_back(D);
-      TmpDecls.clear();
-    } else if (Tok.is(tok::pound_if)) {
+    if (Tok.is(tok::pound_if)) {
       auto IfConfigResult = parseIfConfig(
         [&](SmallVectorImpl<ASTNode> &Elements, bool IsActive) {
           parseBraceItems(Elements, Kind, IsActive
                             ? BraceItemListKind::ActiveConditionalBlock
                             : BraceItemListKind::InactiveConditionalBlock);
         });
-      
-      if (IfConfigResult.isParseError()) {
-        NeedParseErrorRecovery = true;
-        continue;
-      }
-      
-      Result = IfConfigResult.get();
-      
-      if (!Result) {
-        NeedParseErrorRecovery = true;
-        continue;
-      }
+      if (auto ICD = IfConfigResult.getPtrOrNull()) {
+        Result = ICD;
+        // Add the #if block itself
+        Entries.push_back(ICD);
 
-      // Add the #if block itself
-      Entries.push_back(Result);
-
-      IfConfigDecl *ICD = cast<IfConfigDecl>(Result.get<Decl*>());
-      for (auto &Entry : ICD->getActiveClauseElements()) {
-        if (Entry.is<Decl*>() && isa<IfConfigDecl>(Entry.get<Decl*>()))
-          // Don't hoist nested '#if'.
-          continue;
-        Entries.push_back(Entry);
-        if (Entry.is<Decl*>()) {
-          Entry.get<Decl*>()->setEscapedFromIfConfig(true);
+        for (auto &Entry : ICD->getActiveClauseElements()) {
+          if (Entry.is<Decl *>() && isa<IfConfigDecl>(Entry.get<Decl *>()))
+            // Don't hoist nested '#if'.
+            continue;
+          Entries.push_back(Entry);
+          if (Entry.is<Decl *>())
+            Entry.get<Decl *>()->setEscapedFromIfConfig(true);
         }
+      } else {
+        NeedParseErrorRecovery = true;
+        continue;
       }
     } else if (Tok.is(tok::pound_line)) {
       ParserStatus Status = parseLineDirective(true);
@@ -352,6 +326,21 @@ ParserStatus Parser::parseBraceItems(SmallVectorImpl<ASTNode> &Entries,
       ParserStatus Status = parseLineDirective(false);
       BraceItemsStatus |= Status;
       NeedParseErrorRecovery = Status.isError();
+    } else if (isStartOfDecl()) {
+      SmallVector<Decl*, 8> TmpDecls;
+      ParserResult<Decl> DeclResult = 
+          parseDecl(IsTopLevel ? PD_AllowTopLevel : PD_Default,
+                    [&](Decl *D) {TmpDecls.push_back(D);});
+      if (DeclResult.isParseError()) {
+        NeedParseErrorRecovery = true;
+        if (DeclResult.hasCodeCompletion() && IsTopLevel &&
+            isCodeCompletionFirstPass()) {
+          consumeDecl(BeginParserPosition, None, IsTopLevel);
+          return DeclResult;
+        }
+      }
+      Result = DeclResult.getPtrOrNull();
+      Entries.append(TmpDecls.begin(), TmpDecls.end());
     } else if (IsTopLevel) {
       // If this is a statement or expression at the top level of the module,
       // Parse it as a child of a TopLevelCodeDecl.

test/Parse/ConditionalCompilation/pound-if-top-level-3.swift

@@ -2,5 +2,6 @@
 
 #if arch(x86_64)
 // expected-error@+2{{expected '{' in protocol type}}
-// expected-error@+1{{expected #else or #endif at end of conditional compilation block}}
+// expected-error@+1{{extraneous '}' at top level}}
 public protocol CS}
+// expected-error@+1{{expected #else or #endif at end of conditional compilation block}}