[Parse] Cleanup #if directive parsing

rintaro · rintaro · commit c8d717e5f4a8 · 2017-05-16T20:52:46.000+09:00
Now that, IfConfigDecl holds ASTNode regardless statements position or
declarations postion. We can construct it in single method.
diff --git a/include/swift/Parse/Parser.h b/include/swift/Parse/Parser.h
@@ -716,9 +716,10 @@ class Parser {
   ParserResult<TypeDecl> parseDeclAssociatedType(ParseDeclOptions Flags,
                                                  DeclAttributes &Attributes);
   
-  ParserResult<IfConfigDecl> parseDeclIfConfig(ParseDeclOptions Flags);
-  ParserResult<IfConfigDecl> parseStmtIfConfig(BraceItemListKind Kind
-                                                 = BraceItemListKind::Brace);
+  /// Parse a #if ... #endif directive.
+  /// Delegate callback function to parse elements in the blocks.
+  ParserResult<IfConfigDecl> parseIfConfig(
+    llvm::function_ref<void(SmallVectorImpl<ASTNode> &, bool)> parseElements);
 
   /// Parse a #line/#sourceLocation directive.
   /// 'isLine = true' indicates parsing #line instead of #sourcelocation
diff --git a/lib/Parse/ParseDecl.cpp b/lib/Parse/ParseDecl.cpp
@@ -2100,7 +2100,30 @@ Parser::parseDecl(ParseDeclOptions Flags,
   }
 
   if (Tok.is(tok::pound_if)) {
-    auto IfConfigResult = parseDeclIfConfig(Flags);
+    auto IfConfigResult = parseIfConfig(
+      [&](SmallVectorImpl<ASTNode> &Decls, bool IsActive) {
+        Optional<Scope> scope;
+        if (!IsActive)
+          scope.emplace(this, getScopeInfo().getCurrentScope()->getKind(),
+                        /*inactiveConfigBlock=*/true);
+
+        ParserStatus Status;
+        bool PreviousHadSemi = true;
+        while (Tok.isNot(tok::pound_else, tok::pound_endif, tok::pound_elseif,
+                         tok::eof)) {
+          if (Tok.is(tok::r_brace)) {
+            diagnose(Tok.getLoc(),
+                      diag::unexpected_rbrace_in_conditional_compilation_block);
+            // If we see '}', following declarations don't look like belong to
+            // the current decl context; skip them.
+            skipUntilConditionalBlockClose();
+            break;
+          }
+          Status |= parseDeclItem(PreviousHadSemi, Flags,
+                                  [&](Decl *D) {Decls.emplace_back(D);});
+        }
+      });
+
     if (auto ICD = IfConfigResult.getPtrOrNull()) {
       // The IfConfigDecl is ahead of its members in source order.
       Handler(ICD);
diff --git a/lib/Parse/ParseIfConfig.cpp b/lib/Parse/ParseIfConfig.cpp
@@ -549,129 +549,74 @@ static bool isVersionIfConfigCondition(Expr *Condition) {
 
 } // end anonymous namespace
 
-/// Parse and populate a list of #if/#elseif/#else/#endif clauses.
+/// Parse and populate a #if ... #endif directive.
 /// Delegate callback function to parse elements in the blocks.
-template <unsigned N>
-static ParserStatus parseIfConfig(
-    Parser &P, SmallVectorImpl<IfConfigClause> &Clauses,
-    SourceLoc &EndLoc, bool HadMissingEnd,
+ParserResult<IfConfigDecl> Parser::parseIfConfig(
     llvm::function_ref<void(SmallVectorImpl<ASTNode> &, bool)> parseElements) {
+
+  SmallVector<IfConfigClause, 4> Clauses;
   Parser::StructureMarkerRAII ParsingDecl(
-      P, P.Tok.getLoc(), Parser::StructureMarkerKind::IfConfig);
+      *this, Tok.getLoc(), Parser::StructureMarkerKind::IfConfig);
 
   bool foundActive = false;
   bool isVersionCondition = false;
   while (1) {
-    bool isElse = P.Tok.is(tok::pound_else);
-    SourceLoc ClauseLoc = P.consumeToken();
+    bool isElse = Tok.is(tok::pound_else);
+    SourceLoc ClauseLoc = consumeToken();
     Expr *Condition = nullptr;
     bool isActive = false;
 
     // Parse and evaluate the directive.
     if (isElse) {
       isActive = !foundActive;
     } else {
-      llvm::SaveAndRestore<bool> S(P.InPoundIfEnvironment, true);
-      ParserResult<Expr> Result = P.parseExprSequence(diag::expected_expr,
+      llvm::SaveAndRestore<bool> S(InPoundIfEnvironment, true);
+      ParserResult<Expr> Result = parseExprSequence(diag::expected_expr,
                                                       /*isBasic*/true,
                                                       /*isForDirective*/true);
       if (Result.isNull())
         return makeParserError();
       Condition = Result.get();
-      if (validateIfConfigCondition(Condition, P.Context, P.Diags)) {
+      if (validateIfConfigCondition(Condition, Context, Diags)) {
         // Error in the condition;
         isActive = false;
         isVersionCondition = false;
       } else if (!foundActive) {
         // Evaludate the condition only if we haven't found any active one.
-        isActive = evaluateIfConfigCondition(Condition, P.Context);
+        isActive = evaluateIfConfigCondition(Condition, Context);
         isVersionCondition = isVersionIfConfigCondition(Condition);
       }
     }
 
     foundActive |= isActive;
 
-    if (!P.Tok.isAtStartOfLine() && P.Tok.isNot(tok::eof)) {
-      P.diagnose(P.Tok.getLoc(),
-                 diag::extra_tokens_conditional_compilation_directive);
+    if (!Tok.isAtStartOfLine() && Tok.isNot(tok::eof)) {
+      diagnose(Tok.getLoc(),
+               diag::extra_tokens_conditional_compilation_directive);
     }
 
     // Parse elements
-    SmallVector<ASTNode, N> Elements;
+    SmallVector<ASTNode, 16> Elements;
     if (isActive || !isVersionCondition) {
       parseElements(Elements, isActive);
     } else {
-      DiagnosticTransaction DT(P.Diags);
-      P.skipUntilConditionalBlockClose();
+      DiagnosticTransaction DT(Diags);
+      skipUntilConditionalBlockClose();
       DT.abort();
     }
 
     Clauses.emplace_back(ClauseLoc, Condition,
-                         P.Context.AllocateCopy(Elements), isActive);
+                         Context.AllocateCopy(Elements), isActive);
 
-    if (P.Tok.isNot(tok::pound_elseif, tok::pound_else))
+    if (Tok.isNot(tok::pound_elseif, tok::pound_else))
       break;
 
     if (isElse)
-      P.diagnose(P.Tok, diag::expected_close_after_else_directive);
+      diagnose(Tok, diag::expected_close_after_else_directive);
   }
 
-  HadMissingEnd = P.parseEndIfDirective(EndLoc);
-  return makeParserSuccess();
-}
-
-/// Parse #if ... #endif in declarations position.
-ParserResult<IfConfigDecl> Parser::parseDeclIfConfig(ParseDeclOptions Flags) {
-  SmallVector<IfConfigClause, 4> Clauses;
-  SourceLoc EndLoc;
-  bool HadMissingEnd = false;
-  auto Status = parseIfConfig<8>(
-      *this, Clauses, EndLoc, HadMissingEnd,
-      [&](SmallVectorImpl<ASTNode> &Decls, bool IsActive) {
-    Optional<Scope> scope;
-    if (!IsActive)
-      scope.emplace(this, getScopeInfo().getCurrentScope()->getKind(),
-                    /*inactiveConfigBlock=*/true);
-
-    ParserStatus Status;
-    bool PreviousHadSemi = true;
-    while (Tok.isNot(tok::pound_else, tok::pound_endif, tok::pound_elseif,
-                      tok::eof)) {
-      if (Tok.is(tok::r_brace)) {
-        diagnose(Tok.getLoc(),
-                  diag::unexpected_rbrace_in_conditional_compilation_block);
-        // If we see '}', following declarations don't look like belong to
-        // the current decl context; skip them.
-        skipUntilConditionalBlockClose();
-        break;
-      }
-      Status |= parseDeclItem(PreviousHadSemi, Flags,
-                              [&](Decl *D) {Decls.emplace_back(D);});
-    }
-  });
-  if (Status.isError())
-    return makeParserErrorResult<IfConfigDecl>();
-
-  IfConfigDecl *ICD = new (Context) IfConfigDecl(CurDeclContext,
-                                                 Context.AllocateCopy(Clauses),
-                                                 EndLoc, HadMissingEnd);
-  return makeParserResult(ICD);
-}
-
-/// Parse #if ... #endif in statements position.
-ParserResult<IfConfigDecl> Parser::parseStmtIfConfig(BraceItemListKind Kind) {
-  SmallVector<IfConfigClause, 4> Clauses;
   SourceLoc EndLoc;
-  bool HadMissingEnd = false;
-  auto Status = parseIfConfig<16>(
-      *this, Clauses, EndLoc, HadMissingEnd,
-      [&](SmallVectorImpl<ASTNode> &Elements, bool IsActive) {
-    parseBraceItems(Elements, Kind, IsActive
-                      ? BraceItemListKind::ActiveConditionalBlock
-                      : BraceItemListKind::InactiveConditionalBlock);
-  });
-  if (Status.isError())
-    return makeParserErrorResult<IfConfigDecl>();
+  bool HadMissingEnd = parseEndIfDirective(EndLoc);
 
   auto *ICD = new (Context) IfConfigDecl(CurDeclContext,
                                          Context.AllocateCopy(Clauses),
diff --git a/lib/Parse/ParseStmt.cpp b/lib/Parse/ParseStmt.cpp
@@ -293,7 +293,12 @@ ParserStatus Parser::parseBraceItems(SmallVectorImpl<ASTNode> &Entries,
         Entries.push_back(D);
       TmpDecls.clear();
     } else if (Tok.is(tok::pound_if)) {
-      auto IfConfigResult = parseStmtIfConfig(Kind);
+      auto IfConfigResult = parseIfConfig(
+        [&](SmallVectorImpl<ASTNode> &Elements, bool IsActive) {
+          parseBraceItems(Elements, Kind, IsActive
+                            ? BraceItemListKind::ActiveConditionalBlock
+                            : BraceItemListKind::InactiveConditionalBlock);
+        });
       
       if (IfConfigResult.isParseError()) {
         NeedParseErrorRecovery = true;
