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