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

[Fred Fu via Phabricator via cfe-commits]

capfredf marked an inline comment as not done.
capfredf added inline comments.


================
Comment at: clang/lib/Interpreter/CodeCompletion.cpp:84
+  std::string AllCodeText =
+      MainInterp.getAllInput() + "\nvoid dummy(){\n" + Buffer.str() + "}";
+  auto Lines = std::count(AllCodeText.begin(), AllCodeText.end(), '\n') + 1;
----------------
capfredf wrote:
> v.g.vassilev wrote:
> > I am not sure why we need to wrap this code. Can we teach the `TopLevelStmtDecl` to work with the code completion logic?
> I think the problem has less to do with the TopLevelStmtDecl. Usually, a `TopLevelStmtDecl` instance is a successful result from `ParseXXXXX`,  but when `CodeCompletion` is triggered during parsing, nothing meaning comes out from `ParseXXXX`.  
> 
> We need to wrap our input because we need `Sema::CodeCompleteOrdinaryName` to work on code from the REPL in a different completion context than the same code from a regular source file.
> 
> In a regular C++ file, 
> 
> ```
> int foo = 42;
> f_
> ```
> 
> Since top-level expressions are not supported, `foo` should not be an option. 
> 
> But in a REPL session,
> 
> ```
> clang-repl> int foo = 42;
> clang-repl> f_
> ```
> 
> we are allowed to use `foo` to make a statement like `foo + 84;`.
> 
> Since the argument for `CompletionContext` to `CodeCompleteOrdinaryName` is controlled by the parsing process, ie. there is no way to configure it from outside, I think faking context is a straightforward solution. 
> 
> Alternatively, we can check if the compiler is in incremental mode and call `CodeCompleteOrdinaryName` with the context we want in parsing. Something like:
> 
> ```
> if (PP.isIncrementalProcessingEnabled() {
>   Actions.CodeCompleteOrdinaryName(getCurScope(), Sema::PCC_Statement);
> } else {
>   Actions.CodeCompleteOrdinaryName(getCurScope(), Sema::PCC_Namespace);
> }
> ```
> 
> I personally think both solutions are extensionally equivalent, but the alternative is a bit intrusive. 
I think the combination of checking `isIncrementalProcessingEnabled` and adding a new PCC might be better since it is really a different context at the REPL


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154382