junaire marked 16 inline comments as done. junaire added inline comments.
================ Comment at: clang/include/clang/Interpreter/Interpreter.h:72 + llvm::Error ParseAndExecute(llvm::StringRef Code, Value *V = nullptr); + llvm::Expected<llvm::JITTargetAddress> CompileDtorCall(CXXRecordDecl *CXXRD); ---------------- aaron.ballman wrote: > Hopefully this function isn't intending to mutate the passed object? Unfortunately `Sema::LookupDestructor` ask for a non-const pointer... (https://github.com/llvm/llvm-project/blob/26e164fada3721b0939e6b7c2dfe1793d95229bb/clang/lib/Sema/SemaLookup.cpp#L3625) ================ Comment at: clang/include/clang/Interpreter/Interpreter.h:102 + + Expr *SynthesizeExpr(clang::Expr *E); + ---------------- aaron.ballman wrote: > Any chance we can make this const correct as well? (I'll stop asking in this > review -- can you take a pass through it to add const correctness where it > isn't obnoxiously viral to do so?) FindRuntimeInterface mutates ValuePrintingInfo vector so it can't be const afaict. ================ Comment at: clang/include/clang/Interpreter/Value.h:17 + +#include <cstdint> + ---------------- aaron.ballman wrote: > Unused include can be removed. uintptr_t comes from this header so it's not unused. ================ Comment at: clang/include/clang/Interpreter/Value.h:77 + + K_Void, + K_PtrOrObj, ---------------- aaron.ballman wrote: > Fixing indentation It's already formatted by clang-format and if I change this arc doesn't even allow me to upload the diff. I believe it's a bug in clang-format and I have filed https://github.com/llvm/llvm-project/issues/60535 So I'd like to fix this when I commit this :) ================ Comment at: clang/lib/Interpreter/Interpreter.cpp:534 + } + llvm_unreachable("Unknown runtime interface kind"); + } ---------------- sgraenitz wrote: > Is this a leftover from an old `default` label? there's no default label since we handle all the cases here. But indeed it looks a bit ugly and I removed it. ================ Comment at: clang/lib/Interpreter/Value.cpp:91 + static constexpr unsigned char Canary[8] = {0x4c, 0x37, 0xad, 0x8f, + 0x2d, 0x23, 0x95, 0x91}; +}; ---------------- sgraenitz wrote: > Can we add a comment with an explanation of the magic numbers please? I'm uncertain if these numbers actually have specific meanings, I just copied them from Cling directly. 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