v.g.vassilev added a comment.
I think we are getting there. I added a few suggestions how to improve the
current design.
================
Comment at: clang/include/clang/Interpreter/CodeCompletion.h:15
+#define LLVM_CLANG_INTERPRETER_CODE_COMPLETION_H
+#include "llvm/LineEditor/LineEditor.h"
+
----------------
I still am a bit uncomfortable with relying on a line editor api to address a
generic question one could as the interpreter such as "What's visible at that
code position"?
Can we not make the overloaded operators outside of the `ReplListCompleter`
class. That means moving them in ClangRepl.cpp?
================
Comment at: clang/include/clang/Interpreter/Interpreter.h:42
class IncrementalParser;
+class CodeCompleteConsumer;
----------------
We should put that in its alphabetical order.
================
Comment at: clang/lib/Interpreter/CodeCompletion.cpp:111
+ auto *CConsumer = new ReplCompletionConsumer(Results);
+ auto Interp = Interpreter::createForCodeCompletion(
+ CB, MainInterp.getCompilerInstance(), CConsumer);
----------------
I do not believe we need a spe
================
Comment at: clang/lib/Interpreter/IncrementalParser.cpp:324
-llvm::Expected<PartialTranslationUnit &>
-IncrementalParser::Parse(llvm::StringRef input) {
+llvm::Error IncrementalParser::ParseForCodeCompletion(llvm::StringRef input,
+ size_t Col, size_t Line) {
----------------
Is there any other particular reason for this routine to be different rather
than `PP.SetCodeCompletionPoint(*FE, Line, Col)`? That is, can we merge back
the old code and only set the point of completion externally?
================
Comment at: clang/lib/Interpreter/Interpreter.cpp:304
+llvm::Expected<std::unique_ptr<Interpreter>>
+Interpreter::createForCodeCompletion(
+ IncrementalCompilerBuilder &CB, const CompilerInstance *ParentCI,
----------------
capfredf wrote:
> v.g.vassilev wrote:
> > I still do not entirely understand why we can "just" ask the codecompletion
> > infrastructure for the possible options at the current position. I know
> > that's probably easier set than done but I'd like to entertain that idea
> > for a while..
> I'm a bit confused. Can you expand on the suggestion?
I do not believe we need a special case for creating a CodeCompletionConsumer.
I think we could rely on the logic in
`CompilerInstance::createCodeCompletionConsumer` which can be triggered in
`IncrementalAction::ExecuteAction` just like in
`ASTFrontendAction::ExecuteAction`. We will need to copy some of the code over
but then you could start the same interpreter infrastructure with slightly
different flags.
================
Comment at: clang/lib/Parse/ParseDecl.cpp:6652
+ break;
+ }
+
----------------
Can we test this change separately outside of clang-repl? That would mean
creating a tests and run it with `clang -Xclang -fincremental-extensions`
================
Comment at: clang/lib/Parse/Parser.cpp:934
+ PCC = Sema::PCC_Namespace;
+ };
Actions.CodeCompleteOrdinaryName(
----------------
Likewise, it would be great if we have a test in clang-only mode.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D154382/new/
https://reviews.llvm.org/D154382
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
