[PATCH] D141215: [clang-repl] Introduce Value to capture expression results

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added inline comments.


================
Comment at: clang/include/clang/Interpreter/Interpreter.h:97
+
+  enum InterfaceKind { NoAlloc, WithAlloc, CopyArray };
+
----------------
junaire wrote:
> v.g.vassilev wrote:
> > This can probably go in the RuntimeInterfaceBuilder class.
> We need it. See:
> 
> ```
> class RuntimeInterfaceBuilder
>     : public TypeVisitor<RuntimeInterfaceBuilder, Interpreter::InterfaceKind> {
>    ...
> }
> ```
Can't this be an enum which is file local?


================
Comment at: clang/lib/Interpreter/Interpreter.cpp:350
   std::list<PartialTranslationUnit> &PTUs = IncrParser->getPTUs();
-  if (N > PTUs.size())
+  if (N + InitPTUSize > PTUs.size())
     return llvm::make_error<llvm::StringError>("Operation failed. "
----------------
junaire wrote:
> v.g.vassilev wrote:
> > v.g.vassilev wrote:
> > > I'd propose `IncrParser->getPTUs()` to return the list starting from `InitPTUSize`. That should solve the issue you see.
> > Ping.
> I'm uncertain about how to do this. Can you elaborate?
The `std::list<PartialTranslationUnit> PTUs;` in `IncrementalParser.h` stores the all PTUs that we saw, including the Interpreter builtins which contain declarations required by the Interpreter to run. I'd propose  to make `PTUs` of type `std::vector` and `getPTUs()` to return an `ArrayRef` starting from `PTUs.begin() + N` where `N` is the number of builtins that we must not undo.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141215