junaire marked 6 inline comments as done.
junaire added inline comments.

================
Comment at: clang/include/clang/Interpreter/Interpreter.h:67
   create(std::unique_ptr<CompilerInstance> CI);
+  ASTContext &getASTContext() const;
   const CompilerInstance *getCompilerInstance() const;
----------------
v.g.vassilev wrote:
> We should not need this. `CompileDtorCall` can probably do 
> `RD->getASTContext()` instead.
This is a really convenient helper, I'd proposed to keep it.


================
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:
> > 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.
> OK, return ArrayRef sounds good to me.
ArrayRef doesn't fit our needs, using getEffectivePTUSize instead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141215

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to