Merge pull request #6005 from rintaro/parse-decl-semicolon

slavapestov · web-flow · commit 4dbaae21f6cf · 2017-01-17T12:26:46.000-08:00
[Parse] Split declaration list parsing from parseList()
diff --git a/include/swift/AST/Decl.h b/include/swift/AST/Decl.h
@@ -744,8 +744,6 @@ class alignas(1 << DeclAlignInBits) Decl {
   /// Returns the source range of the entire declaration.
   SourceRange getSourceRange() const;
 
-  SourceLoc TrailingSemiLoc;
-
   LLVM_ATTRIBUTE_DEPRECATED(
       void dump() const LLVM_ATTRIBUTE_USED,
       "only for use within the debugger");
diff --git a/include/swift/AST/Expr.h b/include/swift/AST/Expr.h
@@ -499,8 +499,6 @@ class alignas(8) Expr {
   SourceLoc getLoc() const { return (SUBEXPR)->getLoc(); } \
   SourceRange getSourceRange() const { return (SUBEXPR)->getSourceRange(); }
 
-  SourceLoc TrailingSemiLoc;
-
   /// getSemanticsProvidingExpr - Find the smallest subexpression
   /// which obeys the property that evaluating it is exactly
   /// equivalent to evaluating this expression.
diff --git a/include/swift/AST/Stmt.h b/include/swift/AST/Stmt.h
@@ -78,7 +78,6 @@ class alignas(8) Stmt {
   SourceLoc getEndLoc() const;
   
   SourceRange getSourceRange() const;
-  SourceLoc TrailingSemiLoc;
   
   /// isImplicit - Determines whether this statement was implicitly-generated,
   /// rather than explicitly written in the AST.
diff --git a/include/swift/Parse/Parser.h b/include/swift/Parse/Parser.h
@@ -603,9 +603,8 @@ class Parser {
   bool parseMatchingToken(tok K, SourceLoc &TokLoc, Diag<> ErrorDiag,
                           SourceLoc OtherLoc);
 
-  /// \brief Parse the list of statements, expressions, or declarations.
+  /// \brief Parse a comma separated list of some elements.
   ParserStatus parseList(tok RightK, SourceLoc LeftLoc, SourceLoc &RightLoc,
-                         tok SeparatorK, bool OptionalSep,
                          bool AllowSepAfterLast, Diag<> ErrorDiag,
                          std::function<ParserStatus()> callback);
 
@@ -723,15 +722,15 @@ class Parser {
                                            DeclAttributes &Attributes);
   ParserStatus parseInheritance(SmallVectorImpl<TypeLoc> &Inherited,
                                 SourceLoc *classRequirementLoc);
+  bool parseDeclList(SourceLoc LBLoc, SourceLoc &RBLoc,
+                     Diag<> ErrorDiag, ParseDeclOptions Options,
+                     llvm::function_ref<void(Decl*)> handler);
   ParserResult<ExtensionDecl> parseDeclExtension(ParseDeclOptions Flags,
                                                  DeclAttributes &Attributes);
   ParserResult<EnumDecl> parseDeclEnum(ParseDeclOptions Flags,
                                        DeclAttributes &Attributes);
   ParserStatus parseDeclEnumCase(ParseDeclOptions Flags, DeclAttributes &Attributes,
                                  SmallVectorImpl<Decl *> &decls);
-  bool parseNominalDeclMembers(SourceLoc LBLoc, SourceLoc &RBLoc,
-                               Diag<> ErrorDiag, ParseDeclOptions flags,
-                               llvm::function_ref<void(Decl*)> handler);
   ParserResult<StructDecl>
   parseDeclStruct(ParseDeclOptions Flags, DeclAttributes &Attributes);
   ParserResult<ClassDecl>
diff --git a/lib/Parse/ParseDecl.cpp b/lib/Parse/ParseDecl.cpp
@@ -2253,12 +2253,6 @@ ParserStatus Parser::parseDecl(ParseDeclOptions Flags,
       InternalHandler(DeclResult.get());
   }
 
-  if (Tok.is(tok::semi)) {
-    SourceLoc TrailingSemiLoc = consumeToken(tok::semi);
-    if (Status.isSuccess())
-      LastDecl->TrailingSemiLoc = TrailingSemiLoc;
-  }
-
   if (Status.isSuccess()) {
     // If we parsed 'class' or 'static', but didn't handle it above, complain
     // about it.
@@ -2592,6 +2586,61 @@ parseIdentifierDeclName(Parser &P, Identifier &Result, SourceLoc &L,
                                  ResyncP1, Diagnostic(ID, Args...));
 }
 
+/// Parse a Decl item in decl list.
+static ParserStatus parseDeclItem(Parser &P,
+                                  bool &PreviousHadSemi,
+                                  Parser::ParseDeclOptions Options,
+                                  llvm::function_ref<void(Decl*)> handler) {
+  if (P.Tok.is(tok::semi)) {
+    // Consume ';' without preceding decl.
+    P.diagnose(P.Tok, diag::unexpected_separator, ";")
+      .fixItRemove(P.Tok.getLoc());
+    P.consumeToken();
+    // Return success because we already recovered.
+    return makeParserSuccess();
+  }
+
+  // If the previous declaration didn't have a semicolon and this new
+  // declaration doesn't start a line, complain.
+  if (!PreviousHadSemi && !P.Tok.isAtStartOfLine() && !P.Tok.is(tok::unknown)) {
+    auto endOfPrevious = P.getEndOfPreviousLoc();
+    P.diagnose(endOfPrevious, diag::declaration_same_line_without_semi)
+      .fixItInsert(endOfPrevious, ";");
+  }
+
+  ParserStatus Status = P.parseDecl(Options, handler);
+  if (Status.isError())
+    P.skipUntilDeclRBrace(tok::semi, tok::pound_endif);
+  PreviousHadSemi = P.consumeIf(tok::semi);
+  return Status;
+}
+
+/// \brief Parse the members in a struct/class/enum/protocol/extension.
+///
+/// \verbatim
+///    decl* '}'
+/// \endverbatim
+bool Parser::parseDeclList(SourceLoc LBLoc, SourceLoc &RBLoc,
+                           Diag<> ErrorDiag, ParseDeclOptions Options,
+                           llvm::function_ref<void(Decl*)> handler) {
+  ParserStatus Status;
+  bool PreviousHadSemi = true;
+  while (Tok.isNot(tok::r_brace)) {
+    Status |= parseDeclItem(*this, PreviousHadSemi, Options, handler);
+    if (Tok.isAny(tok::eof, tok::pound_endif, tok::pound_else,
+                  tok::pound_elseif)) {
+      IsInputIncomplete = true;
+      break;
+    }
+  }
+
+  parseMatchingToken(tok::r_brace, RBLoc, ErrorDiag, LBLoc);
+
+  // If we found the closing brace, then the caller should not care if there
+  // were errors while parsing inner decls, because we recovered.
+  return !RBLoc.isValid();
+}
+
 /// \brief Parse an 'extension' declaration.
 ///
 /// \verbatim
@@ -2651,20 +2700,15 @@ Parser::parseDeclExtension(ParseDeclOptions Flags, DeclAttributes &Attributes) {
     ContextChange CC(*this, ext);
     Scope S(this, ScopeKind::Extension);
 
-    ParserStatus BodyStatus =
-        parseList(tok::r_brace, LBLoc, RBLoc, tok::semi, /*OptionalSep=*/true,
-                  /*AllowSepAfterLast=*/false, diag::expected_rbrace_extension,
-                  [&]() -> ParserStatus {
-      ParseDeclOptions Options(PD_HasContainerType |
-                               PD_InExtension);
+    ParseDeclOptions Options(PD_HasContainerType | PD_InExtension);
+
+    if (parseDeclList(LBLoc, RBLoc, diag::expected_rbrace_extension,
+                      Options, [&] (Decl *D) {ext->addMember(D);}))
+      status.setIsParseError();
 
-      return parseDecl(Options, [&] (Decl *D) {ext->addMember(D);});
-    });
     // Don't propagate the code completion bit from members: we cannot help
     // code completion inside a member decl, and our callers cannot do
     // anything about it either.  But propagate the error bit.
-    if (BodyStatus.isError())
-      status.setIsParseError();
   }
 
   ext->setBraces({LBLoc, RBLoc});
@@ -2862,14 +2906,20 @@ ParserResult<IfConfigDecl> Parser::parseDeclIfConfig(ParseDeclOptions Flags) {
     SmallVector<Decl*, 8> Decls;
     if (ConfigState.shouldParse()) {
       ParserStatus Status;
-      while (Tok.isNot(tok::pound_else) && Tok.isNot(tok::pound_endif) &&
-             Tok.isNot(tok::pound_elseif)) {
-        Status = parseDecl(Flags, [&](Decl *D) {Decls.push_back(D);});
-        if (Status.isError()) {
-          diagnose(Tok, diag::expected_close_to_if_directive);
+      bool PreviousHadSemi = true;
+      while (Tok.isNot(tok::pound_else, tok::pound_endif, tok::pound_elseif)) {
+        SourceLoc StartLoc = Tok.getLoc();
+        Status |= parseDeclItem(*this, PreviousHadSemi, Flags,
+                                [&](Decl *D) {Decls.push_back(D);});
+        if (StartLoc == Tok.getLoc()) {
+          assert(Status.isError() && "no progress without error?");
           skipUntilConditionalBlockClose();
           break;
         }
+        if (Tok.isAny(tok::eof)) {
+          diagnose(Tok, diag::expected_close_to_if_directive);
+          break;
+        }
       }
     } else {
       DiagnosticTransaction DT(Diags);
@@ -4773,9 +4823,8 @@ ParserResult<EnumDecl> Parser::parseDeclEnum(ParseDeclOptions Flags,
     ContextChange CC(*this, ED);
     Scope S(this, ScopeKind::ClassBody);
     ParseDeclOptions Options(PD_HasContainerType | PD_AllowEnumElement | PD_InEnum);
-    if (parseNominalDeclMembers(LBLoc, RBLoc,
-                                diag::expected_rbrace_enum,
-                                Options, [&] (Decl *D) { ED->addMember(D); }))
+    if (parseDeclList(LBLoc, RBLoc, diag::expected_rbrace_enum,
+                      Options, [&] (Decl *D) { ED->addMember(D); }))
       Status.setIsParseError();
   }
 
@@ -4956,47 +5005,6 @@ ParserStatus Parser::parseDeclEnumCase(ParseDeclOptions Flags,
   return Status;
 }
 
-/// \brief Parse the members in a struct/class/enum/protocol definition.
-///
-/// \verbatim
-///    decl*
-/// \endverbatim
-bool Parser::parseNominalDeclMembers(SourceLoc LBLoc, SourceLoc &RBLoc,
-                                     Diag<> ErrorDiag, ParseDeclOptions flags,
-                                     llvm::function_ref<void(Decl*)> handler) {
-  Decl *lastDecl = nullptr;
-  auto internalHandler = [&](Decl *D) {
-    lastDecl = D;
-    handler(D);
-  };
-  bool previousHadSemi = true;
-  parseList(tok::r_brace, LBLoc, RBLoc, tok::semi, /*OptionalSep=*/true,
-            /*AllowSepAfterLast=*/false, ErrorDiag, [&]() -> ParserStatus {
-    // If the previous declaration didn't have a semicolon and this new
-    // declaration doesn't start a line, complain.
-    if (!previousHadSemi && !Tok.isAtStartOfLine() && !Tok.is(tok::unknown)) {
-      SourceLoc endOfPrevious = getEndOfPreviousLoc();
-      diagnose(endOfPrevious, diag::declaration_same_line_without_semi)
-        .fixItInsert(endOfPrevious, ";");
-      // FIXME: Add semicolon to the AST?
-    }
-
-    previousHadSemi = false;
-    if (parseDecl(flags, internalHandler).isError())
-      return makeParserError();
-
-    // Check whether the previous declaration had a semicolon after it.
-    if (lastDecl && lastDecl->TrailingSemiLoc.isValid())
-      previousHadSemi = true;
-
-    return makeParserSuccess();
-  });
-
-  // If we found the closing brace, then the caller should not care if there
-  // were errors while parsing inner decls, because we recovered.
-  return !RBLoc.isValid();
-}
-
 /// \brief Parse a 'struct' declaration, returning true (and doing no token
 /// skipping) on error.
 ///
@@ -5070,9 +5078,8 @@ ParserResult<StructDecl> Parser::parseDeclStruct(ParseDeclOptions Flags,
     ContextChange CC(*this, SD);
     Scope S(this, ScopeKind::StructBody);
     ParseDeclOptions Options(PD_HasContainerType | PD_InStruct);
-    if (parseNominalDeclMembers(LBLoc, RBLoc,
-                                diag::expected_rbrace_struct,
-                                Options, [&](Decl *D) {SD->addMember(D);}))
+    if (parseDeclList(LBLoc, RBLoc, diag::expected_rbrace_struct,
+                      Options, [&](Decl *D) {SD->addMember(D);}))
       Status.setIsParseError();
   }
 
@@ -5160,8 +5167,8 @@ ParserResult<ClassDecl> Parser::parseDeclClass(SourceLoc ClassLoc,
       if (isa<DestructorDecl>(D))
         CD->setHasDestructor();
     };
-    if (parseNominalDeclMembers(LBLoc, RBLoc, diag::expected_rbrace_class,
-                                Options, Handler))
+    if (parseDeclList(LBLoc, RBLoc, diag::expected_rbrace_class,
+                      Options, Handler))
       Status.setIsParseError();
   }
 
@@ -5259,9 +5266,8 @@ parseDeclProtocol(ParseDeclOptions Flags, DeclAttributes &Attributes) {
       ParseDeclOptions Options(PD_HasContainerType |
                                PD_DisallowInit |
                                PD_InProtocol);
-      if (parseNominalDeclMembers(LBraceLoc, RBraceLoc,
-                                  diag::expected_rbrace_protocol,
-                                  Options, [&](Decl *D) {Proto->addMember(D);}))
+      if (parseDeclList(LBraceLoc, RBraceLoc, diag::expected_rbrace_protocol,
+                        Options, [&](Decl *D) {Proto->addMember(D);}))
         Status.setIsParseError();
     }
 
diff --git a/lib/Parse/ParseExpr.cpp b/lib/Parse/ParseExpr.cpp
@@ -2528,8 +2528,7 @@ ParserStatus Parser::parseExprList(tok leftTok, tok rightTok,
   StructureMarkerRAII ParsingExprList(*this, Tok);
 
   leftLoc = consumeToken(leftTok);
-  ParserStatus status = parseList(rightTok, leftLoc, rightLoc, tok::comma,
-                                  /*OptionalSep=*/false,
+  ParserStatus status = parseList(rightTok, leftLoc, rightLoc,
                                   /*AllowSepAfterLast=*/false,
                                   rightTok == tok::r_paren
                                     ? diag::expected_rparen_expr_list
@@ -2877,7 +2876,6 @@ ParserResult<Expr> Parser::parseExprArray(SourceLoc LSquareLoc,
   CommaLocs.push_back(CommaLoc);
 
   Status |= parseList(tok::r_square, LSquareLoc, RSquareLoc,
-                      tok::comma, /*OptionalSep=*/false,
                       /*AllowSepAfterLast=*/true,
                       diag::expected_rsquare_array_expr,
                       [&] () -> ParserStatus
@@ -2928,8 +2926,8 @@ ParserResult<Expr> Parser::parseExprDictionary(SourceLoc LSquareLoc,
   bool FirstPair = true;
 
   ParserStatus Status =
-      parseList(tok::r_square, LSquareLoc, RSquareLoc, tok::comma,
-                /*OptionalSep=*/false, /*AllowSepAfterLast=*/true,
+      parseList(tok::r_square, LSquareLoc, RSquareLoc,
+                /*AllowSepAfterLast=*/true,
                 diag::expected_rsquare_array_expr, [&]() -> ParserStatus {
     // Parse the next key.
     ParserResult<Expr> Key;
diff --git a/lib/Parse/ParsePattern.cpp b/lib/Parse/ParsePattern.cpp
@@ -159,8 +159,8 @@ Parser::parseParameterClause(SourceLoc &leftParenLoc,
 
   // Parse the parameter list.
   bool isClosure = paramContext == ParameterContextKind::Closure;
-  return parseList(tok::r_paren, leftParenLoc, rightParenLoc, tok::comma,
-                      /*OptionalSep=*/false, /*AllowSepAfterLast=*/false,
+  return parseList(tok::r_paren, leftParenLoc, rightParenLoc,
+                      /*AllowSepAfterLast=*/false,
                       diag::expected_rparen_parameter,
                       [&]() -> ParserStatus {
     ParsedParameter param;
@@ -867,7 +867,7 @@ ParserResult<Pattern> Parser::parsePatternTuple() {
   // Parse all the elements.
   SmallVector<TuplePatternElt, 8> elts;
   ParserStatus ListStatus =
-    parseList(tok::r_paren, LPLoc, RPLoc, tok::comma, /*OptionalSep=*/false,
+    parseList(tok::r_paren, LPLoc, RPLoc,
               /*AllowSepAfterLast=*/false,
               diag::expected_rparen_tuple_pattern_list,
               [&] () -> ParserStatus {
diff --git a/lib/Parse/ParseSIL.cpp b/lib/Parse/ParseSIL.cpp
@@ -1233,7 +1233,6 @@ bool SILParser::parseSILBBArgsAtBranch(SmallVector<SILValue, 6> &Args,
     SourceLoc RParenLoc;
 
     if (P.parseList(tok::r_paren, LParenLoc, RParenLoc,
-                    tok::comma, /*OptionalSep=*/false,
                     /*AllowSepAfterLast=*/false,
                     diag::sil_basicblock_arg_rparen,
                     [&]() -> ParserStatus {
diff --git a/lib/Parse/ParseStmt.cpp b/lib/Parse/ParseStmt.cpp
@@ -311,8 +311,6 @@ ParserStatus Parser::parseBraceItems(SmallVectorImpl<ASTNode> &Entries,
 
       for (Decl *D : TmpDecls)
         Entries.push_back(D);
-      if (!TmpDecls.empty())
-        PreviousHadSemi = TmpDecls.back()->TrailingSemiLoc.isValid();
       TmpDecls.clear();
     } else if (Tok.is(tok::pound_if)) {
       SourceLoc StartLoc = Tok.getLoc();
@@ -415,13 +413,7 @@ ParserStatus Parser::parseBraceItems(SmallVectorImpl<ASTNode> &Entries,
     }
 
     if (!NeedParseErrorRecovery && !PreviousHadSemi && Tok.is(tok::semi)) {
-      if (Result) {
-        if (Result.is<Expr*>()) {
-          Result.get<Expr*>()->TrailingSemiLoc = consumeToken(tok::semi);
-        } else {
-          Result.get<Stmt*>()->TrailingSemiLoc = consumeToken(tok::semi);
-        }
-      }
+      consumeToken();
       PreviousHadSemi = true;
     }
 
diff --git a/lib/Parse/ParseType.cpp b/lib/Parse/ParseType.cpp
@@ -682,7 +682,6 @@ ParserResult<TupleTypeRepr> Parser::parseTypeTupleBody() {
     Labels;
 
   ParserStatus Status = parseList(tok::r_paren, LPLoc, RPLoc,
-                                  tok::comma, /*OptionalSep=*/false,
                                   /*AllowSepAfterLast=*/false,
                                   diag::expected_rparen_tuple_type_list,
                                   [&] () -> ParserStatus {
diff --git a/lib/Parse/Parser.cpp b/lib/Parse/Parser.cpp
diff --git a/test/Parse/ConditionalCompilation/decl_parse_errors.swift b/test/Parse/ConditionalCompilation/decl_parse_errors.swift
diff --git a/test/Parse/trailing-semi.swift b/test/Parse/trailing-semi.swift
