Revert "[ClangRepl] Type Directed Code Completion"

[capfredf]

Reverts #67349

[capfredf]

@vgvassilev

[llvmbot]

@llvm/pr-subscribers-clang

Author: Fred Fu (capfredf)

Changes

Reverts llvm/llvm-project#67349

---

Patch is 24.17 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/73259.diff

6 Files Affected:

• (modified) clang/include/clang/Interpreter/CodeCompletion.h (+3-22)
• (modified) clang/include/clang/Interpreter/Interpreter.h (-1)
• (modified) clang/lib/Interpreter/CodeCompletion.cpp (+23-199)
• (modified) clang/lib/Interpreter/Interpreter.cpp (-4)
• (modified) clang/tools/clang-repl/ClangRepl.cpp (+15-9)
• (modified) clang/unittests/Interpreter/CodeCompletionTest.cpp (+10-209)

diff --git a/clang/include/clang/Interpreter/CodeCompletion.h b/clang/include/clang/Interpreter/CodeCompletion.h
index c64aa899759fd87..9adcdf0dc3afac6 100644
--- a/clang/include/clang/Interpreter/CodeCompletion.h
+++ b/clang/include/clang/Interpreter/CodeCompletion.h
@@ -23,27 +23,8 @@ namespace clang {
 class CodeCompletionResult;
 class CompilerInstance;
 
-struct ReplCodeCompleter {
-  ReplCodeCompleter() = default;
-  std::string Prefix;
-
-  /// \param InterpCI [in] The compiler instance that is used to trigger code
-  /// completion
-
-  /// \param Content [in] The string where code completion is triggered.
-
-  /// \param Line [in] The line number of the code completion point.
-
-  /// \param Col [in] The column number of the code completion point.
-
-  /// \param ParentCI [in] The running interpreter compiler instance that
-  /// provides ASTContexts.
-
-  /// \param CCResults [out] The completion results.
-  void codeComplete(CompilerInstance *InterpCI, llvm::StringRef Content,
-                    unsigned Line, unsigned Col,
-                    const CompilerInstance *ParentCI,
-                    std::vector<std::string> &CCResults);
-};
+void codeComplete(CompilerInstance *InterpCI, llvm::StringRef Content,
+                  unsigned Line, unsigned Col, const CompilerInstance *ParentCI,
+                  std::vector<std::string> &CCResults);
 } // namespace clang
 #endif
diff --git a/clang/include/clang/Interpreter/Interpreter.h b/clang/include/clang/Interpreter/Interpreter.h
index 01858dfcc90ac5a..43573fb1a4b8915 100644
--- a/clang/include/clang/Interpreter/Interpreter.h
+++ b/clang/include/clang/Interpreter/Interpreter.h
@@ -101,7 +101,6 @@ class Interpreter {
   const ASTContext &getASTContext() const;
   ASTContext &getASTContext();
   const CompilerInstance *getCompilerInstance() const;
-  CompilerInstance *getCompilerInstance();
   llvm::Expected<llvm::orc::LLJIT &> getExecutionEngine();
 
   llvm::Expected<PartialTranslationUnit &> Parse(llvm::StringRef Code);
diff --git a/clang/lib/Interpreter/CodeCompletion.cpp b/clang/lib/Interpreter/CodeCompletion.cpp
index c34767c3ef9b87d..c40e11b9d1ece0a 100644
--- a/clang/lib/Interpreter/CodeCompletion.cpp
+++ b/clang/lib/Interpreter/CodeCompletion.cpp
@@ -12,7 +12,6 @@
 
 #include "clang/Interpreter/CodeCompletion.h"
 #include "clang/AST/ASTImporter.h"
-#include "clang/AST/DeclLookups.h"
 #include "clang/AST/DeclarationName.h"
 #include "clang/AST/ExternalASTSource.h"
 #include "clang/Basic/IdentifierTable.h"
@@ -24,8 +23,6 @@
 #include "clang/Sema/CodeCompleteConsumer.h"
 #include "clang/Sema/CodeCompleteOptions.h"
 #include "clang/Sema/Sema.h"
-#include "llvm/Support/Debug.h"
-#define DEBUG_TYPE "REPLCC"
 
 namespace clang {
 
@@ -42,15 +39,11 @@ clang::CodeCompleteOptions getClangCompleteOpts() {
 
 class ReplCompletionConsumer : public CodeCompleteConsumer {
 public:
-  ReplCompletionConsumer(std::vector<std::string> &Results,
-                         ReplCodeCompleter &CC)
+  ReplCompletionConsumer(std::vector<std::string> &Results)
       : CodeCompleteConsumer(getClangCompleteOpts()),
         CCAllocator(std::make_shared<GlobalCodeCompletionAllocator>()),
-        CCTUInfo(CCAllocator), Results(Results), CC(CC) {}
+        CCTUInfo(CCAllocator), Results(Results){};
 
-  // The entry of handling code completion. When the function is called, we
-  // create a `Context`-based handler (see classes defined below) to handle each
-  // completion result.
   void ProcessCodeCompleteResults(class Sema &S, CodeCompletionContext Context,
                                   CodeCompletionResult *InResults,
                                   unsigned NumResults) final;
@@ -63,146 +56,26 @@ class ReplCompletionConsumer : public CodeCompleteConsumer {
   std::shared_ptr<GlobalCodeCompletionAllocator> CCAllocator;
   CodeCompletionTUInfo CCTUInfo;
   std::vector<std::string> &Results;
-  ReplCodeCompleter &CC;
-};
-
-/// The class CompletionContextHandler contains four interfaces, each of
-/// which handles one type of completion result.
-/// Its derived classes are used to create concrete handlers based on
-/// \c CodeCompletionContext.
-class CompletionContextHandler {
-protected:
-  CodeCompletionContext CCC;
-  std::vector<std::string> &Results;
-
-private:
-  Sema &S;
-
-public:
-  CompletionContextHandler(Sema &S, CodeCompletionContext CCC,
-                           std::vector<std::string> &Results)
-      : CCC(CCC), Results(Results), S(S) {}
-
-  /// Converts a Declaration completion result to a completion string, and then
-  /// stores it in Results.
-  virtual void handleDeclaration(const CodeCompletionResult &Result) {
-    auto PreferredType = CCC.getPreferredType();
-    if (PreferredType.isNull()) {
-      Results.push_back(Result.Declaration->getName().str());
-      return;
-    }
-
-    if (auto *VD = dyn_cast<VarDecl>(Result.Declaration)) {
-      auto ArgumentType = VD->getType();
-      if (PreferredType->isReferenceType()) {
-        QualType RT = PreferredType->castAs<ReferenceType>()->getPointeeType();
-        Sema::ReferenceConversions RefConv;
-        Sema::ReferenceCompareResult RefRelationship =
-            S.CompareReferenceRelationship(SourceLocation(), RT, ArgumentType,
-                                           &RefConv);
-        switch (RefRelationship) {
-        case Sema::Ref_Compatible:
-        case Sema::Ref_Related:
-          Results.push_back(VD->getName().str());
-          break;
-        case Sema::Ref_Incompatible:
-          break;
-        }
-      } else if (S.Context.hasSameType(ArgumentType, PreferredType)) {
-        Results.push_back(VD->getName().str());
-      }
-    }
-  }
-
-  /// Converts a Keyword completion result to a completion string, and then
-  /// stores it in Results.
-  virtual void handleKeyword(const CodeCompletionResult &Result) {
-    auto Prefix = S.getPreprocessor().getCodeCompletionFilter();
-    // Add keyword to the completion results only if we are in a type-aware
-    // situation.
-    if (!CCC.getBaseType().isNull() || !CCC.getPreferredType().isNull())
-      return;
-    if (StringRef(Result.Keyword).startswith(Prefix))
-      Results.push_back(Result.Keyword);
-  }
-
-  /// Converts a Pattern completion result to a completion string, and then
-  /// stores it in Results.
-  virtual void handlePattern(const CodeCompletionResult &Result) {}
-
-  /// Converts a Macro completion result to a completion string, and then stores
-  /// it in Results.
-  virtual void handleMacro(const CodeCompletionResult &Result) {}
-};
-
-class DotMemberAccessHandler : public CompletionContextHandler {
-public:
-  DotMemberAccessHandler(Sema &S, CodeCompletionContext CCC,
-                         std::vector<std::string> &Results)
-      : CompletionContextHandler(S, CCC, Results) {}
-  void handleDeclaration(const CodeCompletionResult &Result) override {
-    auto *ID = Result.Declaration->getIdentifier();
-    if (!ID)
-      return;
-    if (!isa<CXXMethodDecl>(Result.Declaration))
-      return;
-    const auto *Fun = cast<CXXMethodDecl>(Result.Declaration);
-    if (Fun->getParent()->getCanonicalDecl() ==
-        CCC.getBaseType()->getAsCXXRecordDecl()->getCanonicalDecl()) {
-      LLVM_DEBUG(llvm::dbgs() << "[In HandleCodeCompleteDOT] Name : "
-                              << ID->getName() << "\n");
-      Results.push_back(ID->getName().str());
-    }
-  }
-
-  void handleKeyword(const CodeCompletionResult &Result) override {}
 };
 
 void ReplCompletionConsumer::ProcessCodeCompleteResults(
     class Sema &S, CodeCompletionContext Context,
     CodeCompletionResult *InResults, unsigned NumResults) {
-
-  auto Prefix = S.getPreprocessor().getCodeCompletionFilter();
-  CC.Prefix = Prefix;
-
-  std::unique_ptr<CompletionContextHandler> CCH;
-
-  // initialize fine-grained code completion handler based on the code
-  // completion context.
-  switch (Context.getKind()) {
-  case CodeCompletionContext::CCC_DotMemberAccess:
-    CCH.reset(new DotMemberAccessHandler(S, Context, this->Results));
-    break;
-  default:
-    CCH.reset(new CompletionContextHandler(S, Context, this->Results));
-  };
-
-  for (unsigned I = 0; I < NumResults; I++) {
+  for (unsigned I = 0; I < NumResults; ++I) {
     auto &Result = InResults[I];
     switch (Result.Kind) {
     case CodeCompletionResult::RK_Declaration:
-      if (Result.Hidden) {
-        break;
-      }
-      if (!Result.Declaration->getDeclName().isIdentifier() ||
-          !Result.Declaration->getName().startswith(Prefix)) {
-        break;
+      if (auto *ID = Result.Declaration->getIdentifier()) {
+        Results.push_back(ID->getName().str());
       }
-      CCH->handleDeclaration(Result);
       break;
     case CodeCompletionResult::RK_Keyword:
-      CCH->handleKeyword(Result);
-      break;
-    case CodeCompletionResult::RK_Macro:
-      CCH->handleMacro(Result);
+      Results.push_back(Result.Keyword);
       break;
-    case CodeCompletionResult::RK_Pattern:
-      CCH->handlePattern(Result);
+    default:
       break;
     }
   }
-
-  std::sort(Results.begin(), Results.end());
 }
 
 class IncrementalSyntaxOnlyAction : public SyntaxOnlyAction {
@@ -245,16 +118,6 @@ void IncrementalSyntaxOnlyAction::ExecuteAction() {
   CI.getASTContext().getTranslationUnitDecl()->setHasExternalVisibleStorage(
       true);
 
-  // Load all external decls into current context. Under the hood, it calls
-  // ExternalSource::completeVisibleDeclsMap, which make all decls on the redecl
-  // chain visible.
-  //
-  // This is crucial to code completion on dot members, since a bound variable
-  // before "." would be otherwise treated out-of-scope.
-  //
-  // clang-repl> Foo f1;
-  // clang-repl> f1.<tab>
-  CI.getASTContext().getTranslationUnitDecl()->lookups();
   SyntaxOnlyAction::ExecuteAction();
 }
 
@@ -271,7 +134,6 @@ ExternalSource::ExternalSource(ASTContext &ChildASTCtxt, FileManager &ChildFM,
 
 bool ExternalSource::FindExternalVisibleDeclsByName(const DeclContext *DC,
                                                     DeclarationName Name) {
-
   IdentifierTable &ParentIdTable = ParentASTCtxt.Idents;
 
   auto ParentDeclName =
@@ -297,67 +159,29 @@ void ExternalSource::completeVisibleDeclsMap(
   for (auto *DeclCtxt = ParentTUDeclCtxt; DeclCtxt != nullptr;
        DeclCtxt = DeclCtxt->getPreviousDecl()) {
     for (auto &IDeclContext : DeclCtxt->decls()) {
-      if (!llvm::isa<NamedDecl>(IDeclContext))
-        continue;
-
-      NamedDecl *Decl = llvm::cast<NamedDecl>(IDeclContext);
-
-      auto DeclOrErr = Importer->Import(Decl);
-      if (!DeclOrErr) {
-        // if an error happens, it usually means the decl has already been
-        // imported or the decl is a result of a failed import.  But in our
-        // case, every import is fresh each time code completion is
-        // triggered. So Import usually doesn't fail. If it does, it just means
-        // the related decl can't be used in code completion and we can safely
-        // drop it.
-        llvm::consumeError(DeclOrErr.takeError());
-        continue;
-      }
-
-      if (!llvm::isa<NamedDecl>(*DeclOrErr))
-        continue;
-
-      NamedDecl *importedNamedDecl = llvm::cast<NamedDecl>(*DeclOrErr);
-
-      SetExternalVisibleDeclsForName(ChildDeclContext,
-                                     importedNamedDecl->getDeclName(),
-                                     importedNamedDecl);
-
-      if (!llvm::isa<CXXRecordDecl>(importedNamedDecl))
-        continue;
-
-      auto *Record = llvm::cast<CXXRecordDecl>(importedNamedDecl);
-
-      if (auto Err = Importer->ImportDefinition(Decl)) {
-        // the same as above
-        consumeError(std::move(Err));
-        continue;
+      if (NamedDecl *Decl = llvm::dyn_cast<NamedDecl>(IDeclContext)) {
+        if (auto DeclOrErr = Importer->Import(Decl)) {
+          if (NamedDecl *importedNamedDecl =
+                  llvm::dyn_cast<NamedDecl>(*DeclOrErr)) {
+            SetExternalVisibleDeclsForName(ChildDeclContext,
+                                           importedNamedDecl->getDeclName(),
+                                           importedNamedDecl);
+          }
+
+        } else {
+          llvm::consumeError(DeclOrErr.takeError());
+        }
       }
-
-      Record->setHasLoadedFieldsFromExternalStorage(true);
-      LLVM_DEBUG(llvm::dbgs()
-                 << "\nCXXRecrod : " << Record->getName() << " size(methods): "
-                 << std::distance(Record->method_begin(), Record->method_end())
-                 << " has def?:  " << Record->hasDefinition()
-                 << " # (methods): "
-                 << std::distance(Record->getDefinition()->method_begin(),
-                                  Record->getDefinition()->method_end())
-                 << "\n");
-      for (auto *Meth : Record->methods())
-        SetExternalVisibleDeclsForName(ChildDeclContext, Meth->getDeclName(),
-                                       Meth);
     }
     ChildDeclContext->setHasExternalLexicalStorage(false);
   }
 }
 
-void ReplCodeCompleter::codeComplete(CompilerInstance *InterpCI,
-                                     llvm::StringRef Content, unsigned Line,
-                                     unsigned Col,
-                                     const CompilerInstance *ParentCI,
-                                     std::vector<std::string> &CCResults) {
+void codeComplete(CompilerInstance *InterpCI, llvm::StringRef Content,
+                  unsigned Line, unsigned Col, const CompilerInstance *ParentCI,
+                  std::vector<std::string> &CCResults) {
   auto DiagOpts = DiagnosticOptions();
-  auto consumer = ReplCompletionConsumer(CCResults, *this);
+  auto consumer = ReplCompletionConsumer(CCResults);
 
   auto diag = InterpCI->getDiagnosticsPtr();
   std::unique_ptr<ASTUnit> AU(ASTUnit::LoadFromCompilerInvocationAction(
diff --git a/clang/lib/Interpreter/Interpreter.cpp b/clang/lib/Interpreter/Interpreter.cpp
index c9fcef5b5b5af13..7968c62cbd3e7b3 100644
--- a/clang/lib/Interpreter/Interpreter.cpp
+++ b/clang/lib/Interpreter/Interpreter.cpp
@@ -319,10 +319,6 @@ const CompilerInstance *Interpreter::getCompilerInstance() const {
   return IncrParser->getCI();
 }
 
-CompilerInstance *Interpreter::getCompilerInstance() {
-  return IncrParser->getCI();
-}
-
 llvm::Expected<llvm::orc::LLJIT &> Interpreter::getExecutionEngine() {
   if (!IncrExecutor) {
     if (auto Err = CreateExecutor())
diff --git a/clang/tools/clang-repl/ClangRepl.cpp b/clang/tools/clang-repl/ClangRepl.cpp
index 98ba143a9492a61..5663c2c5a6c9285 100644
--- a/clang/tools/clang-repl/ClangRepl.cpp
+++ b/clang/tools/clang-repl/ClangRepl.cpp
@@ -15,8 +15,6 @@
 #include "clang/Frontend/FrontendDiagnostic.h"
 #include "clang/Interpreter/CodeCompletion.h"
 #include "clang/Interpreter/Interpreter.h"
-#include "clang/Lex/Preprocessor.h"
-#include "clang/Sema/Sema.h"
 
 #include "llvm/ExecutionEngine/Orc/LLJIT.h"
 #include "llvm/LineEditor/LineEditor.h"
@@ -125,14 +123,22 @@ ReplListCompleter::operator()(llvm::StringRef Buffer, size_t Pos,
 
     return {};
   }
-  auto *MainCI = (*Interp)->getCompilerInstance();
-  auto CC = clang::ReplCodeCompleter();
-  CC.codeComplete(MainCI, Buffer, Lines, Pos + 1,
-                  MainInterp.getCompilerInstance(), Results);
+
+  codeComplete(
+      const_cast<clang::CompilerInstance *>((*Interp)->getCompilerInstance()),
+      Buffer, Lines, Pos + 1, MainInterp.getCompilerInstance(), Results);
+
+  size_t space_pos = Buffer.rfind(" ");
+  llvm::StringRef Prefix;
+  if (space_pos == llvm::StringRef::npos) {
+    Prefix = Buffer;
+  } else {
+    Prefix = Buffer.substr(space_pos + 1);
+  }
+
   for (auto c : Results) {
-    if (c.find(CC.Prefix) == 0)
-      Comps.push_back(
-          llvm::LineEditor::Completion(c.substr(CC.Prefix.size()), c));
+    if (c.find(Prefix) == 0)
+      Comps.push_back(llvm::LineEditor::Completion(c.substr(Prefix.size()), c));
   }
   return Comps;
 }
diff --git a/clang/unittests/Interpreter/CodeCompletionTest.cpp b/clang/unittests/Interpreter/CodeCompletionTest.cpp
index cd7fdfa588a5d04..8f5f3545029d087 100644
--- a/clang/unittests/Interpreter/CodeCompletionTest.cpp
+++ b/clang/unittests/Interpreter/CodeCompletionTest.cpp
@@ -1,9 +1,7 @@
 #include "clang/Interpreter/CodeCompletion.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Interpreter/Interpreter.h"
-#include "clang/Lex/Preprocessor.h"
 #include "clang/Sema/CodeCompleteConsumer.h"
-#include "clang/Sema/Sema.h"
 #include "llvm/LineEditor/LineEditor.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/raw_ostream.h"
@@ -21,7 +19,7 @@ static std::unique_ptr<Interpreter> createInterpreter() {
 }
 
 static std::vector<std::string> runComp(clang::Interpreter &MainInterp,
-                                        llvm::StringRef Input,
+                                        llvm::StringRef Prefix,
                                         llvm::Error &ErrR) {
   auto CI = CB.CreateCpp();
   if (auto Err = CI.takeError()) {
@@ -39,14 +37,16 @@ static std::vector<std::string> runComp(clang::Interpreter &MainInterp,
 
   std::vector<std::string> Results;
   std::vector<std::string> Comps;
-  auto *MainCI = (*Interp)->getCompilerInstance();
-  auto CC = ReplCodeCompleter();
-  CC.codeComplete(MainCI, Input, /* Lines */ 1, Input.size() + 1,
-                  MainInterp.getCompilerInstance(), Results);
+
+  codeComplete(
+      const_cast<clang::CompilerInstance *>((*Interp)->getCompilerInstance()),
+      Prefix, /* Lines */ 1, Prefix.size(), MainInterp.getCompilerInstance(),
+      Results);
 
   for (auto Res : Results)
-    if (Res.find(CC.Prefix) == 0)
+    if (Res.find(Prefix) == 0)
       Comps.push_back(Res);
+
   return Comps;
 }
 
@@ -62,9 +62,8 @@ TEST(CodeCompletionTest, Sanity) {
   }
   auto Err = llvm::Error::success();
   auto comps = runComp(*Interp, "f", Err);
-  EXPECT_EQ((size_t)2, comps.size()); // float and foo
-  EXPECT_EQ(comps[0], std::string("float"));
-  EXPECT_EQ(comps[1], std::string("foo"));
+  EXPECT_EQ((size_t)2, comps.size()); // foo and float
+  EXPECT_EQ(comps[0], std::string("foo"));
   EXPECT_EQ((bool)Err, false);
 }
 
@@ -111,202 +110,4 @@ TEST(CodeCompletionTest, CompFunDeclsNoError) {
   EXPECT_EQ((bool)Err, false);
 }
 
-TEST(CodeCompletionTest, TypedDirected) {
-  auto Interp = createInterpreter();
-  if (auto R = Interp->ParseAndExecute("int application = 12;")) {
-    consumeError(std::move(R));
-    return;
-  }
-  if (auto R = Interp->ParseAndExecute("char apple = '2';")) {
-    consumeError(std::move(R));
-    return;
-  }
-  if (auto R = Interp->ParseAndExecute("void add(int &SomeInt){}")) {
-    consumeError(std::move(R));
-    return;
-  }
-  {
-    auto Err = llvm::Error::success();
-    auto comps = runComp(*Interp, std::string("add("), Err);
-    EXPECT_EQ((size_t)1, comps.size());
-    EXPECT_EQ((bool)Err, false);
-  }
-
-  if (auto R = Interp->ParseAndExecute("int banana = 42;")) {
-    consumeError(std::move(R));
-    return;
-  }
-
-  {
-    auto Err = llvm::Error::success();
-    auto comps = runComp(*Interp, std::string("add("), Err);
-    EXPECT_EQ((size_t)2, comps.size());
-    EXPECT_EQ(comps[0], "application");
-    EXPECT_EQ(comps[1], "banana");
-    EXPECT_EQ((bool)Err, false);
-  }
-
-  {
-    auto Err = llvm::Error::success();
-    auto comps = runComp(*Interp, std::string("add(b"), Err);
-    EXPECT_EQ((size_t)1, comps.size());
-    EXPECT_EQ(comps[0], "banana");
-    EXPECT_EQ((bool)Err, false);
-  }
-}
-
-TEST(CodeCompletionTest, SanityClasses) {
-  auto Interp = createInterpreter();
-  if (auto R = Interp->ParseAndExecute("struct Apple{};")) {
-    consumeError(std::move(R));
-    return;
-  }
-  if (auto R = Interp->ParseAndExecute("void takeApple(Apple &a1){}")) {
-    consumeError(std::move(R));
-    return;
-  }
-  if (auto R = Interp->ParseAndExecute("Apple a1;")) {
-    consumeError(std::move(R));
-    return;
-  }
-  if (auto R = Interp->ParseAndExecute("void takeAppleCopy(Apple a1){}")) {
-    consumeError(std::move(R));
-    return;
-  }
-
-  {
-    auto Err = llvm::Error::success();
-    auto comps = runComp(*Interp, "takeApple(", Err);
-    EXPECT_EQ((size_t)1, comps.size());
-    EXPECT_EQ(comps[0], std::string("a1"));
-    EXPECT_EQ((bool)Err, false);
-  }
-  {
-    auto Err = llvm::Error::success();
-    auto comps = runComp(*Interp, std::string("takeAppleCopy("), Err);
-    EXPECT_E...
[truncated]