v.g.vassilev added inline comments.
================
Comment at: clang/lib/Interpreter/IncrementalParser.h:86
+ Parser &getParser() { return *P; }
+
----------------
Is that used?
================
Comment at: clang/lib/Interpreter/ValuePrinter.cpp:262
+
+static llvm::Expected<llvm::orc::ExecutorAddr> CompileDecl(Interpreter &Interp,
+ Decl *D) {
----------------
Let's add a FIXME here. The problem `CompileDecl` and `GenModule` intends to
solve is that when we synthesize AST we need to inform the rest of the Clang
infrastructure about it and attach the produced `llvm::Module` to the JIT so
that it can resolve symbols from it.
In cling that is solved with a RAII object which marks a scope where the
synthesis happens and takes care to inform the rest of the infrastructure. We
need something similar and a little more robust maybe. Something like
`ASTSynthesisRAII` which ultimately hides this machinery especially from the
public API.
================
Comment at: clang/lib/Interpreter/ValuePrinter.cpp:442
+
+REPL_EXTERNAL_VISIBILITY std::string PrintValueRuntime(const char *const *Val)
{
+ return PrintString(Val);
----------------
All of the `PrintValueRuntime` seem to be duplicating code. We could use the
preprocessor to expand these to but it would require some changes which I
believe could be done as a separate patch:
* Perhaps we should be compatible with cling here in terms of naming as that's
a public API - there I believe we use `printValue`.
* We should try to leverage `TemplateBase.cpp::printIntegral` for integral
types.
* We should not return `std::string` here but a `const char*` and we should
provide somewhere storage for them. That would probably make porting to
embedded systems easier since on many the <string> header is hard to support.
I believe it would be enough to have a fixme for now.
================
Comment at: clang/lib/Interpreter/ValuePrinter.cpp:473
+ else if (auto *BT = DesugaredTy.getCanonicalType()->getAs<BuiltinType>()) {
+ switch (BT->getKind()) {
+ case BuiltinType::Bool: {
----------------
We could use the preprocessor the way we do in `Value.cpp` to expand this
dispatcher.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146809/new/
https://reviews.llvm.org/D146809
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
