v.g.vassilev added a comment.
Overall it looks like we are heading in a good direction. Here are some initial
comments from my partial review.
================
Comment at: clang/include/clang/Interpreter/Interpreter.h:109
+ std::list<PartialTranslationUnit> &getPTUs();
+ std::unique_ptr<llvm::Module> GenModule();
+
----------------
Do we need to expose this function to the outside world?
================
Comment at: clang/lib/Interpreter/ASTHelpers.cpp:1
+#include "ASTHelpers.h"
+
----------------
Likewise.
================
Comment at: clang/lib/Interpreter/ASTHelpers.h:1
+#ifndef LLVM_CLANG_INTERPRETER_AST_HELPERS_H
+#define LLVM_CLANG_INTERPRETER_AST_HELPERS_H
----------------
We are missing the standard llvm header preamble. Clang seems to have slight
preference to using `Utils` in the name. How about renaming this file to
`InterpreterUtils.h`
================
Comment at: clang/lib/Interpreter/IncrementalParser.cpp:22
#include "clang/FrontendTool/Utils.h"
+#include "clang/Interpreter/Interpreter.h"
#include "clang/Parse/Parser.h"
----------------
This introduces a layering violation. Can we avoid it?
================
Comment at: clang/lib/Interpreter/IncrementalParser.cpp:162
+ if (P->getCurToken().is(tok::annot_input_end)) {
+ P->ConsumeAnyToken();
// FIXME: Clang does not call ExitScope on finalizing the regular TU, we
----------------
Why `ConsumeToken` is not sufficient for an `annot_input_end`?
================
Comment at: clang/lib/Interpreter/IncrementalParser.h:82
+
+ CodeGenerator *GetCodeGen() const;
+ std::unique_ptr<llvm::Module> GenModule();
----------------
Do we need to expose CodeGen in this patch?
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
