[PATCH] D137487: [clang][Interp] Start implementing builtin functions

[Erich Keane via Phabricator via cfe-commits]

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