v.g.vassilev added a comment.
Thanks for working on this. Here are some more comments.
================
Comment at: clang/include/clang/Interpreter/Interpreter.h:72
+ void Restore(PartialTranslationUnit &PTU);
+
----------------
I am not sure if that's the best function. We
================
Comment at: clang/lib/Interpreter/IncrementalParser.cpp:281
+void IncrementalParser::Restore(PartialTranslationUnit &PTU) {
+ TranslationUnitDecl *MostRecentTU = PTU.TUPart;
----------------
I think we can merge this function with `Undo` where we can conditionally check
if we have a llvm::Module to recover.
================
Comment at: clang/lib/Interpreter/Interpreter.cpp:273
+ llvm::Error Err = llvm::Error::success();
+ IncrExecutor = std::make_unique<IncrementalExecutor>(*TSCtx, Err, Triple);
+
----------------
I am not sure how feasible is this usecase but in `-fsyntax-only` mode we can
still need undo. That means somebody created an Interpreter instance for
frontend-only operations, such as lookups or template instantiations. In that
case I think it also would make sense to support the undo operation.
================
Comment at: clang/lib/Interpreter/Interpreter.cpp:278
+ }
+ if (N > IncrExecutor->getAvailableModuleSize())
+ return llvm::make_error<llvm::StringError>("Operation failed, "
----------------
In some cases we can have PTU's that do not generate code. For example, `define
A 1` will not produce a corresponding llvm::Module. I think here we should
count the size of the PartialTranslationUnitDecls.
================
Comment at: clang/test/Interpreter/code-undo.cpp:7
+// RUN: cat %s | clang-repl | FileCheck %s
+extern "C" int printf(const char *, ...);
+int x1 = 42;
----------------
Can you add here something like `int j = 0` and print its value after the
`undo`. This way we will make sure we do not undo too much.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D126682/new/
https://reviews.llvm.org/D126682
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
