erichkeane 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); ---------------- This is an unrelated change, perhaps could be in an NFC commit. ================ 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()) { ---------------- `directly` reads oddly here? Do you mean the 'immediately' definition of this? ================ Comment at: clang/lib/AST/Interp/Interp.h:1355 + // Evaluate builtin functions directly. + if (unsigned BID = Func->getBuiltinID()) { + if (InterpretBuiltin(S, PC, BID)) { ---------------- Rather than the 'if'/'else' pair, do we just want to immediately return on the failed builtin? ================ Comment at: clang/lib/AST/Interp/Interp.h:1357 + if (InterpretBuiltin(S, PC, BID)) { + NewFrame.release(); + return true; ---------------- What is going on here? Why is this not a leak? 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