[PATCH] D154382: [ClangRepl] support code completion at a REPL

[Fred Fu via Phabricator via cfe-commits]

capfredf added inline comments.


================
Comment at: clang/include/clang/Frontend/ASTUnit.h:901
+                    SmallVectorImpl<const llvm::MemoryBuffer *> &OwnedBuffers,
+                    std::function<void(CompilerInstance&)> AfterBeginSourceFile = [](CompilerInstance& CI) -> void {});
 
----------------
sammccall wrote:
> capfredf wrote:
> > @v.g.vassilev  Not sure if this is the right solution or idiom to extend this method.  
> > 
> > we can make this high order function more general, like `std::unique_ptr<SyntaxOnlyAction*><()>`
> There's no need for this to be a SyntaxOnlyAction, you can take FrontendAction here.
> 
> I don't think there's any need to provide a factory, it's safe to assume it will always create one action. (We have FrontendActionFactory in tooling, and it's a pain.)
 Ideally, this method requires a FrontendAction with `hasCodeCompletionSupport` returning `true`.  The reason I chose  `SyntaxOnlyAction` are 1. the old code used it 2.  `SyntaxOnlyAction::hasCodeCompletionSupport` returns `true`.  Certainly, right now `SyntaxOnlyAction` does not prevent its subclasses from overriding this method. 


================
Comment at: clang/include/clang/Sema/CodeCompleteConsumer.h:342
+    /// Code completion at a top level in a REPL session.
+    CCC_ReplTopLevel,
   };
----------------
sammccall wrote:
> v.g.vassilev wrote:
> > 
> I don't think this name fits with the others, it describes the client rather than the grammatical/semantic context.
> 
> I would suggest maybe `CCC_TopLevelOrExpression`?
Nice. This one sounds better indeed. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154382