v.g.vassilev added inline comments.
================
Comment at: clang/include/clang/Interpreter/Interpreter.h:39
+class Parser;
+class CodeGenerator;
class CompilerInstance;
----------------
v.g.vassilev wrote:
> We probably do not need these forward declarations.
Looks like they came back.
================
Comment at: clang/include/clang/Interpreter/Interpreter.h:67
create(std::unique_ptr<CompilerInstance> CI);
+ ASTContext &getASTContext() const;
const CompilerInstance *getCompilerInstance() const;
----------------
We should not need this. `CompileDtorCall` can probably do
`RD->getASTContext()` instead.
================
Comment at: clang/include/clang/Interpreter/Interpreter.h:104
+
+ Expr *SynthesizeExpr(clang::Expr *E);
+
----------------
Let's make this file static.
================
Comment at: clang/include/clang/Interpreter/Value.h:1
+//===--- Interpreter.h - Incremental Compiation and Execution---*- C++ -*-===//
+//
----------------
`Value.h` instead of `Interpreter.h`
================
Comment at: clang/include/clang/Interpreter/Value.h:17
+
+#include "llvm/Support/Compiler.h"
+#include <cassert>
----------------
We should probably remove this include as it seems unneeded.
================
Comment at: clang/lib/Interpreter/IncrementalASTConsumer.cpp:37
+
+void IncrementalASTConsumer::Transform(DeclGroupRef &DGR) {
+ for (Decl *D : DGR) {
----------------
Let's inline this function in its caller.
================
Comment at: clang/lib/Interpreter/IncrementalASTConsumer.cpp:43
+ // Flip the flag.
+ TSD->setValuePrinting(false);
+ }
----------------
Do we need to do this? I'd prefer to keep some history in `TSD`.
================
Comment at: clang/lib/Interpreter/IncrementalASTConsumer.h:25
+
+class IncrementalASTConsumer final : public ASTConsumer {
+ Interpreter &Interp;
----------------
Let's move this class in `IncrementalParser.cpp`. I don't think we need a new
file here just yet.
================
Comment at: clang/lib/Interpreter/IncrementalASTConsumer.h:34
+ bool HandleTopLevelDecl(DeclGroupRef DGR) override final;
+ void HandleTranslationUnit(ASTContext &Ctx) override final;
+ static bool classof(const clang::ASTConsumer *) { return true; }
----------------
We should make all interfaces pass through as all of them are important events
that codegen needs to know about.
================
Comment at: clang/lib/Interpreter/IncrementalParser.h:19
#include "clang/AST/GlobalDecl.h"
-
+#include "clang/Parse/Parser.h"
#include "llvm/ADT/ArrayRef.h"
----------------
We don't need this include if we remove `getParser` which we do not need.
================
Comment at: clang/lib/Interpreter/IncrementalParser.h:87
+private:
+ CodeGenerator *GetCodeGen() const;
};
----------------
That should not be needed with the new implementation in this patch.
================
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. "
----------------
I'd propose `IncrParser->getPTUs()` to return the list starting from
`InitPTUSize`. That should solve the issue you see.
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
