tbaeder added inline comments.
================ Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:1374 const Decl *Callee = E->getCalleeDecl(); - if (const auto *FuncDecl = dyn_cast_or_null<FunctionDecl>(Callee)) { + if (const auto *FuncDecl = dyn_cast_if_present<FunctionDecl>(Callee)) { const Function *Func = getFunction(FuncDecl); ---------------- erichkeane wrote: > This is an unrelated change, perhaps could be in an NFC commit. Right, sure ================ Comment at: clang/lib/AST/Interp/Interp.h:1354 - APValue CallResult; - // Note that we cannot assert(CallResult.hasValue()) here since - // Ret() above only sets the APValue if the curent frame doesn't - // have a caller set. - if (Interpret(S, CallResult)) { - NewFrame.release(); // Frame was delete'd already. - assert(S.Current == FrameBefore); - - // For constructors, check that all fields have been initialized. - if (Func->isConstructor()) { - if (!CheckCtorCall(S, PC, ThisPtr)) - return false; + // Evaluate builtin functions directly. + if (unsigned BID = Func->getBuiltinID()) { ---------------- erichkeane wrote: > `directly` reads oddly here? Do you mean the 'immediately' definition of > this? Yes, that was the purpose. I can reword it. ================ Comment at: clang/lib/AST/Interp/Interp.h:1355 + // Evaluate builtin functions directly. + if (unsigned BID = Func->getBuiltinID()) { + if (InterpretBuiltin(S, PC, BID)) { ---------------- erichkeane wrote: > Rather than the 'if'/'else' pair, do we just want to immediately return on > the failed builtin? Sure, that's possible. I guess this ties into the question of whether to use a different opcode for builtin functions; but so far I have found builtin calls to be too similar to regular ones, esp. since they don't have a special AST node. ================ Comment at: clang/lib/AST/Interp/Interp.h:1357 + if (InterpretBuiltin(S, PC, BID)) { + NewFrame.release(); + return true; ---------------- erichkeane wrote: > What is going on here? Why is this not a leak? The current frame is `delete`d when successfully returning the value of the function, this happens in the `Ret` (and `RetVoid`) function in `Interp.cpp`. No opinion on whether that's the best way to do it though. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137487/new/ https://reviews.llvm.org/D137487 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits