v.g.vassilev updated this revision to Diff 449066.
v.g.vassilev edited the summary of this revision.
v.g.vassilev added a comment.
Herald added subscribers: sstefan1, Anastasia.
Herald added a reviewer: jdoerfert.

Rely on the CXXScopeSpec to detect more accurately if we are parsing a 
constructor declarator.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D127284/new/

https://reviews.llvm.org/D127284

Files:
  clang/include/clang/AST/ASTConsumer.h
  clang/include/clang/Parse/Parser.h
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/CodeGen/ModuleBuilder.cpp
  clang/lib/Interpreter/IncrementalParser.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseTentative.cpp
  clang/lib/Parse/Parser.cpp
  clang/test/Interpreter/disambiguate-decl-stmt.cpp
  clang/test/Interpreter/execute-stmts.cpp
  clang/unittests/Interpreter/InterpreterTest.cpp

Index: clang/unittests/Interpreter/InterpreterTest.cpp
===================================================================
--- clang/unittests/Interpreter/InterpreterTest.cpp
+++ clang/unittests/Interpreter/InterpreterTest.cpp
@@ -120,12 +120,7 @@
 
   // FIXME: Add support for wrapping and running statements.
   auto R2 = Interp->Parse("var1++; printf(\"var1 value %d\\n\", var1);");
-  EXPECT_FALSE(!!R2);
-  using ::testing::HasSubstr;
-  EXPECT_THAT(DiagnosticsOS.str(),
-              HasSubstr("error: unknown type name 'var1'"));
-  auto Err = R2.takeError();
-  EXPECT_EQ("Parsing failed.", llvm::toString(std::move(Err)));
+  EXPECT_TRUE(!!R2);
 }
 
 TEST(InterpreterTest, UndoCommand) {
Index: clang/test/Interpreter/execute-stmts.cpp
===================================================================
--- /dev/null
+++ clang/test/Interpreter/execute-stmts.cpp
@@ -0,0 +1,34 @@
+// REQUIRES: host-supports-jit
+// UNSUPPORTED: system-aix
+// RUN: cat %s | clang-repl -Xcc -Xclang -Xcc  -verify | FileCheck %s
+
+// expected-no-diagnostics
+
+extern "C" int printf(const char*,...);
+
+//template <typename T> T call() { printf("call\n"); return T(); }
+//call<int>();
+//// C: call
+
+int i = 1;
+++i;
+printf("i = %d\n", i);
+// CHECK: i = 2
+
+namespace Ns { void f(){ i++; } }
+Ns::f();
+
+void g() { ++i; }
+g();
+::g();
+
+printf("i = %d\n", i);
+// CHECK-NEXT: i = 5
+
+for (; i > 4; --i) printf("i = %d\n", i);
+// CHECK-NEXT: i = 5
+
+int j = i; printf("j = %d\n", j);
+// CHECK-NEXT: j = 4
+
+%quit
Index: clang/test/Interpreter/disambiguate-decl-stmt.cpp
===================================================================
--- /dev/null
+++ clang/test/Interpreter/disambiguate-decl-stmt.cpp
@@ -0,0 +1,45 @@
+// REQUIRES: host-supports-jit
+// UNSUPPORTED: system-aix
+// RUN: cat %s | clang-repl -Xcc -Xclang -Xcc -verify | FileCheck %s
+
+// expected-no-diagnostics
+
+extern "C" int printf(const char*,...);
+
+// Decls which are hard to disambiguate
+
+// Dtors
+using I = int;
+I x = 10;
+x.I::~I();
+x = 20;
+
+// Ctors
+
+// FIXME: Support deduction guide
+// template<typename T> struct A { A(); A(T); };
+// A() -> A<int>;
+
+namespace N { struct S { S(); }; }
+N::S::S() { printf("N::S::S()\n"); }
+N::S s;
+// CHECK: N::S::S()
+
+namespace Ns {namespace Ns { void Ns(); void Fs();}}
+void Ns::Ns::Ns() { printf("void Ns::Ns::Ns()\n"); }
+void Ns::Ns::Fs() {}
+
+Ns::Ns::Fs();
+Ns::Ns::Ns();
+// CHECK-NEXT: void Ns::Ns::Ns()
+
+struct Attrs1 { Attrs1(); };
+Attrs1::Attrs1() __attribute((pure)) = default;
+
+struct Attrs2 { Attrs2(); };
+__attribute((pure)) Attrs2::Attrs2() = default;
+
+// Extra semicolon
+namespace N {};
+
+%quit
Index: clang/lib/Parse/Parser.cpp
===================================================================
--- clang/lib/Parse/Parser.cpp
+++ clang/lib/Parse/Parser.cpp
@@ -582,13 +582,14 @@
 ///
 /// Note that in C, it is an error if there is no first declaration.
 bool Parser::ParseFirstTopLevelDecl(DeclGroupPtrTy &Result,
-                                    Sema::ModuleImportState &ImportState) {
+                                    Sema::ModuleImportState &ImportState,
+                                    StmtVector *Stmts) {
   Actions.ActOnStartOfTranslationUnit();
 
   // For C++20 modules, a module decl must be the first in the TU.  We also
   // need to track module imports.
   ImportState = Sema::ModuleImportState::FirstDecl;
-  bool NoTopLevelDecls = ParseTopLevelDecl(Result, ImportState);
+  bool NoTopLevelDecls = ParseTopLevelDecl(Result, ImportState, Stmts);
 
   // C11 6.9p1 says translation units must have at least one top-level
   // declaration. C++ doesn't have this restriction. We also don't want to
@@ -609,7 +610,8 @@
 ///           declaration
 /// [C++20]   module-import-declaration
 bool Parser::ParseTopLevelDecl(DeclGroupPtrTy &Result,
-                               Sema::ModuleImportState &ImportState) {
+                               Sema::ModuleImportState &ImportState,
+                               StmtVector *Stmts /*=nullptr*/) {
   DestroyTemplateIdAnnotationsRAIIObj CleanupRAII(*this);
 
   // Skip over the EOF token, flagging end of previous input for incremental
@@ -724,6 +726,20 @@
   ParsedAttributes attrs(AttrFactory);
   MaybeParseCXX11Attributes(attrs);
 
+  // FIXME: Remove the incremental processing pre-condition and verify clang
+  // still can pass its test suite, which will harden `isDeclarationStatement`.
+  // Parse a block of top-level-stmts.
+  while (PP.isIncrementalProcessingEnabled() && Stmts &&
+         !isDeclarationStatement(/*DisambiguatingWithExpression=*/true)) {
+    ParsedStmtContext SubStmtCtx = ParsedStmtContext();
+    auto R = ParseStatementOrDeclaration(*Stmts, SubStmtCtx);
+    if (!R.isUsable())
+      return true;
+    Stmts->push_back(R.get());
+    if (Tok.is(tok::eof))
+      return false;
+  }
+
   Result = ParseExternalDeclaration(attrs);
   // An empty Result might mean a line with ';' or some parsing error, ignore
   // it.
Index: clang/lib/Parse/ParseTentative.cpp
===================================================================
--- clang/lib/Parse/ParseTentative.cpp
+++ clang/lib/Parse/ParseTentative.cpp
@@ -46,7 +46,33 @@
 ///           'using' 'namespace' '::'[opt] nested-name-specifier[opt]
 ///                 namespace-name ';'
 ///
-bool Parser::isCXXDeclarationStatement() {
+bool Parser::isCXXDeclarationStatement(
+    bool DisambiguatingWithExpression /*=false*/) {
+  if (DisambiguatingWithExpression) {
+    switch (Tok.getKind()) {
+    case tok::semi:
+      return true; // Not a stmt but can be extra terminator in `namespace{};`
+    case tok::kw_template:
+      return true; // FIXME: What about stmts such as `t::template f<int>();`
+    case tok::kw_inline:
+      return true;
+    case tok::identifier:
+      if (getLangOpts().CPlusPlus) {
+        if (isConstructorDeclarator(/*Unqualified=*/false,
+                                    /*DeductionGuide=*/false,
+                                    /*DisambiguatingWithExpression=*/true))
+          return true;
+        // Check if this is a dtor.
+        if (!TryAnnotateTypeOrScopeToken() && Tok.is(tok::annot_cxxscope) &&
+            NextToken().is(tok::tilde))
+          return true;
+        break;
+      }
+    default:
+      break;
+    }
+  }
+
   switch (Tok.getKind()) {
     // asm-definition
   case tok::kw_asm:
Index: clang/lib/Parse/ParseDecl.cpp
===================================================================
--- clang/lib/Parse/ParseDecl.cpp
+++ clang/lib/Parse/ParseDecl.cpp
@@ -5513,7 +5513,9 @@
   }
 }
 
-bool Parser::isConstructorDeclarator(bool IsUnqualified, bool DeductionGuide) {
+bool Parser::isConstructorDeclarator(
+    bool IsUnqualified, bool DeductionGuide,
+    bool DisambiguatingWithExpression /*=false*/) {
   TentativeParsingAction TPA(*this);
 
   // Parse the C++ scope specifier.
@@ -5525,10 +5527,12 @@
     return false;
   }
 
+  IdentifierInfo *CtorII = nullptr;
   // Parse the constructor name.
   if (Tok.is(tok::identifier)) {
     // We already know that we have a constructor name; just consume
     // the token.
+    CtorII = Tok.getIdentifierInfo();
     ConsumeToken();
   } else if (Tok.is(tok::annot_template_id)) {
     ConsumeAnnotationToken();
@@ -5548,12 +5552,31 @@
   }
   ConsumeParen();
 
-  // A right parenthesis, or ellipsis followed by a right parenthesis signals
-  // that we have a constructor.
-  if (Tok.is(tok::r_paren) ||
-      (Tok.is(tok::ellipsis) && NextToken().is(tok::r_paren))) {
-    TPA.Revert();
-    return true;
+  if (SS.isNotEmpty() && SS.getScopeRep()) {
+    NestedNameSpecifier *NNS = SS.getScopeRep();
+    if (NNS->getAsNamespace() || NNS->getAsNamespaceAlias()) {
+      TPA.Revert();
+      return false;
+    }
+    if (CtorII) {
+      if (NamedDecl *CXXRD = NNS->getAsRecordDecl()) {
+        bool IsCtor = CXXRD->getIdentifier() == CtorII;
+        TPA.Revert();
+        return IsCtor;
+      }
+    }
+  }
+
+  // FIXME: This case is only valid in a member declarator contexts where we
+  // declare a constructor in the class body.
+  if (!DisambiguatingWithExpression) {
+    // A right parenthesis, or ellipsis followed by a right parenthesis signals
+    // that we have a constructor.
+    if (Tok.is(tok::r_paren) ||
+        (Tok.is(tok::ellipsis) && NextToken().is(tok::r_paren))) {
+      TPA.Revert();
+      return true;
+    }
   }
 
   // A C++11 attribute here signals that we have a constructor, and is an
Index: clang/lib/Interpreter/IncrementalParser.cpp
===================================================================
--- clang/lib/Interpreter/IncrementalParser.cpp
+++ clang/lib/Interpreter/IncrementalParser.cpp
@@ -168,11 +168,17 @@
 
   Parser::DeclGroupPtrTy ADecl;
   Sema::ModuleImportState ImportState;
-  for (bool AtEOF = P->ParseFirstTopLevelDecl(ADecl, ImportState); !AtEOF;
-       AtEOF = P->ParseTopLevelDecl(ADecl, ImportState)) {
+  Parser::StmtVector Stmts;
+  for (bool AtEOF = P->ParseFirstTopLevelDecl(ADecl, ImportState, &Stmts);
+       !AtEOF; AtEOF = P->ParseTopLevelDecl(ADecl, ImportState, &Stmts)) {
     // If we got a null return and something *was* parsed, ignore it.  This
     // is due to a top-level semicolon, an action override, or a parse error
     // skipping something.
+    if (!Stmts.empty() && !Consumer->HandleTopLevelStmts(Stmts))
+      return llvm::make_error<llvm::StringError>("Parsing failed. "
+                                                 "The consumer rejected a stmt",
+                                                 std::error_code());
+
     if (ADecl && !Consumer->HandleTopLevelDecl(ADecl.get()))
       return llvm::make_error<llvm::StringError>("Parsing failed. "
                                                  "The consumer rejected a decl",
Index: clang/lib/CodeGen/ModuleBuilder.cpp
===================================================================
--- clang/lib/CodeGen/ModuleBuilder.cpp
+++ clang/lib/CodeGen/ModuleBuilder.cpp
@@ -175,7 +175,16 @@
       Builder->HandleCXXStaticMemberVarInstantiation(VD);
     }
 
+    bool
+    HandleTopLevelStmts(const llvm::SmallVectorImpl<Stmt *> &Stmts) override {
+      if (Diags.hasErrorOccurred())
+        return false; // Abort parsing.
+
+      return Builder->HandleTopLevelStmts(Stmts);
+    }
+
     bool HandleTopLevelDecl(DeclGroupRef DG) override {
+      // FIXME: Why not return false and abort parsing?
       if (Diags.hasErrorOccurred())
         return true;
 
Index: clang/lib/CodeGen/CodeGenModule.h
===================================================================
--- clang/lib/CodeGen/CodeGenModule.h
+++ clang/lib/CodeGen/CodeGenModule.h
@@ -1074,6 +1074,9 @@
   /// Tell the consumer that this variable has been instantiated.
   void HandleCXXStaticMemberVarInstantiation(VarDecl *VD);
 
+  /// Tell the consumer that we have statements on the global scope to process.
+  bool HandleTopLevelStmts(const llvm::SmallVectorImpl<Stmt *> &Stmts);
+
   /// If the declaration has internal linkage but is inside an
   /// extern "C" linkage specification, prepare to emit an alias for it
   /// to the expected name.
Index: clang/lib/CodeGen/CodeGenModule.cpp
===================================================================
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -5100,6 +5100,35 @@
   EmitTopLevelDecl(VD);
 }
 
+bool CodeGenModule::HandleTopLevelStmts(
+    const llvm::SmallVectorImpl<Stmt *> &Stmts) {
+  // FIXME: We should reimplement this in a proper way where we append the
+  // statements to the global init function.
+  static unsigned id = 0;
+  ASTContext &C = getContext();
+  TranslationUnitDecl *TUDecl = C.getTranslationUnitDecl();
+
+  IdentifierInfo *name = &C.Idents.get("_stmts" + std::to_string(id++));
+  SourceLocation noLoc;
+  SourceLocation beginLoc = Stmts[0]->getBeginLoc();
+  FunctionProtoType::ExtProtoInfo EPI;
+  QualType FunctionTy = C.getFunctionType(C.VoidTy, llvm::None, EPI);
+  auto *TSI = C.getTrivialTypeSourceInfo(FunctionTy);
+  FunctionDecl *FD = FunctionDecl::Create(C, TUDecl, beginLoc, noLoc, name,
+                                          FunctionTy, TSI, SC_None);
+
+  CompoundStmt *Body =
+      CompoundStmt::Create(C, Stmts, beginLoc, Stmts.back()->getEndLoc());
+  FD->setBody(Body);
+
+  EmitTopLevelDecl(FD);
+  GlobalDecl GD(FD);
+  auto *Fn = cast<llvm::Function>(GetGlobalValue(getMangledName(GD)));
+  CXXGlobalInits.push_back(Fn);
+
+  return true;
+}
+
 void CodeGenModule::EmitGlobalFunctionDefinition(GlobalDecl GD,
                                                  llvm::GlobalValue *GV) {
   const auto *D = cast<FunctionDecl>(GD.getDecl());
Index: clang/lib/CodeGen/CodeGenAction.cpp
===================================================================
--- clang/lib/CodeGen/CodeGenAction.cpp
+++ clang/lib/CodeGen/CodeGenAction.cpp
@@ -237,6 +237,10 @@
 
       return true;
     }
+    bool
+    HandleTopLevelStmts(const llvm::SmallVectorImpl<Stmt *> &Stmts) override {
+      return Gen->HandleTopLevelStmts(Stmts);
+    }
 
     void HandleInlineFunctionDefinition(FunctionDecl *D) override {
       PrettyStackTraceDecl CrashInfo(D, SourceLocation(),
Index: clang/include/clang/Parse/Parser.h
===================================================================
--- clang/include/clang/Parse/Parser.h
+++ clang/include/clang/Parse/Parser.h
@@ -462,6 +462,10 @@
 
   typedef Sema::FullExprArg FullExprArg;
 
+  /// A SmallVector of statements, with stack size 32 (as that is the only one
+  /// used.)
+  typedef SmallVector<Stmt *, 32> StmtVector;
+
   // Parsing methods.
 
   /// Initialize - Warm up the parser.
@@ -470,12 +474,14 @@
 
   /// Parse the first top-level declaration in a translation unit.
   bool ParseFirstTopLevelDecl(DeclGroupPtrTy &Result,
-                              Sema::ModuleImportState &ImportState);
+                              Sema::ModuleImportState &ImportState,
+                              StmtVector *Stmts = nullptr);
 
   /// ParseTopLevelDecl - Parse one top-level declaration. Returns true if
   /// the EOF was encountered.
   bool ParseTopLevelDecl(DeclGroupPtrTy &Result,
-                         Sema::ModuleImportState &ImportState);
+                         Sema::ModuleImportState &ImportState,
+                         StmtVector *Stmts = nullptr);
   bool ParseTopLevelDecl() {
     DeclGroupPtrTy Result;
     Sema::ModuleImportState IS = Sema::ModuleImportState::NotACXX20Module;
@@ -2061,9 +2067,6 @@
   //===--------------------------------------------------------------------===//
   // C99 6.8: Statements and Blocks.
 
-  /// A SmallVector of statements, with stack size 32 (as that is the only one
-  /// used.)
-  typedef SmallVector<Stmt*, 32> StmtVector;
   /// A SmallVector of expressions, with stack size 12 (the maximum used.)
   typedef SmallVector<Expr*, 12> ExprVector;
   /// A SmallVector of types.
@@ -2410,10 +2413,13 @@
 
   /// isDeclarationStatement - Disambiguates between a declaration or an
   /// expression statement, when parsing function bodies.
+  ///
+  /// \param DisambiguatingWithExpression - True to indicate that the purpose of
+  /// this check is to disambiguate between an expression and a declaration.
   /// Returns true for declaration, false for expression.
-  bool isDeclarationStatement() {
+  bool isDeclarationStatement(bool DisambiguatingWithExpression = false) {
     if (getLangOpts().CPlusPlus)
-      return isCXXDeclarationStatement();
+      return isCXXDeclarationStatement(DisambiguatingWithExpression);
     return isDeclarationSpecifier(true);
   }
 
@@ -2440,7 +2446,8 @@
   /// Starting with a scope specifier, identifier, or
   /// template-id that refers to the current class, determine whether
   /// this is a constructor declarator.
-  bool isConstructorDeclarator(bool Unqualified, bool DeductionGuide = false);
+  bool isConstructorDeclarator(bool Unqualified, bool DeductionGuide = false,
+                               bool DisambiguatingWithExpression = false);
 
   /// Specifies the context in which type-id/expression
   /// disambiguation will occur.
@@ -2478,7 +2485,7 @@
   /// isCXXDeclarationStatement - C++-specialized function that disambiguates
   /// between a declaration or an expression statement, when parsing function
   /// bodies. Returns true for declaration, false for expression.
-  bool isCXXDeclarationStatement();
+  bool isCXXDeclarationStatement(bool DisambiguatingWithExpression = false);
 
   /// isCXXSimpleDeclaration - C++-specialized function that disambiguates
   /// between a simple-declaration or an expression-statement.
Index: clang/include/clang/AST/ASTConsumer.h
===================================================================
--- clang/include/clang/AST/ASTConsumer.h
+++ clang/include/clang/AST/ASTConsumer.h
@@ -13,6 +13,10 @@
 #ifndef LLVM_CLANG_AST_ASTCONSUMER_H
 #define LLVM_CLANG_AST_ASTCONSUMER_H
 
+namespace llvm {
+template <typename T> class SmallVectorImpl;
+}
+
 namespace clang {
   class ASTContext;
   class CXXMethodDecl;
@@ -22,6 +26,7 @@
   class ASTMutationListener;
   class ASTDeserializationListener; // layering violation because void* is ugly
   class SemaConsumer; // layering violation required for safe SemaConsumer
+  class Stmt;
   class TagDecl;
   class VarDecl;
   class FunctionDecl;
@@ -46,6 +51,15 @@
   /// ASTContext.
   virtual void Initialize(ASTContext &Context) {}
 
+  /// HandleTopLevelStmts - Handle the specified top-level statements. This is
+  /// called by the parser to process every top-level Stmt* in incremental
+  /// compilation mode.
+  ///
+  /// \returns true to continue parsing, or false to abort parsing.
+  virtual bool HandleTopLevelStmts(const llvm::SmallVectorImpl<Stmt *> &Stmts) {
+    return true;
+  }
+
   /// HandleTopLevelDecl - Handle the specified top-level declaration.  This is
   /// called by the parser to process every top-level Decl*.
   ///
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to