capfredf marked 5 inline comments as done. capfredf added inline comments.
================ Comment at: clang/lib/Interpreter/CodeCompletion.cpp:111 + auto *CConsumer = new ReplCompletionConsumer(Results); + auto Interp = Interpreter::createForCodeCompletion( + CB, MainInterp.getCompilerInstance(), CConsumer); ---------------- v.g.vassilev wrote: > I do not believe we need a spe I am not sure I am following. For a regular interpreter, create Interpreter -> create Incremental Parser -> IncrmentalASTAction triggered -> set a default code completion consumer(PrintCodeCompletionConsumer) to CI if no code consumer has been set -> Sema initialized. Based on this logic, we need to provide our CodeConsumer so that IncrementalASTAction does not create the default, plus we need to a parent CI to a new code completion interpreter. I don't see how we can avoid a special code path for creation ================ 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) { ---------------- v.g.vassilev wrote: > 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? The regular `Parse` does some lexing after parsing, which I don't think is necessary for code completion. Also, this is more or less because, as a functional programmer, I prefer building things using small functions with explicit arguments. I'll try if I can use the old parse with the completion point set and tested. 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