junaire added inline comments.
================
Comment at: clang/include/clang/Interpreter/Interpreter.h:97
+
+ enum InterfaceKind { NoAlloc, WithAlloc, CopyArray };
+
----------------
v.g.vassilev wrote:
> 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?
You can't put this enum inside RuntimeInterfaceBuilder because its declaration
needs it. If you do so, then the above line will report an error since you use
the enum before defined 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. "
----------------
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.
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
