[PATCH] D136936: [clang][Interp] Handle undefined functions better

Timm Bäder via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Nov 1 22:25:17 PDT 2022


tbaeder added inline comments.


================
Comment at: clang/lib/AST/Interp/ByteCodeEmitter.cpp:30
   if (!FuncDecl->isDefined(FuncDecl) ||
-      (!FuncDecl->hasBody() && FuncDecl->willHaveBody()))
-    return nullptr;
+      (FuncDecl->hasBody() && FuncDecl->willHaveBody()))
+    HasBody = false;
----------------
aaron.ballman wrote:
> `hasBody()` returns `true` if any body in the redeclaration chain already has a body, so I'm surprised to see this change -- I don't know if we reset `willHaveBody()` when we give the function a body, but the logic here seems wrong to me.
I was confused by your question, but it's just about `hasBody()` vs `!hasBody()`, right? The old code used a negation and of course that way it makes sense, at least the way I read it. Yes I forgot the negation. :)


================
Comment at: clang/lib/AST/Interp/Program.h:97
   Function *createFunction(const FunctionDecl *Def, Ts &&... Args) {
+    Def = Def->getCanonicalDecl();
     auto *Func = new Function(*this, Def, std::forward<Ts>(Args)...);
----------------
aaron.ballman wrote:
> Are you trying to get the `FunctionDecl` which represents the function definition, or do you really mean you want the first declaration in the redecl chain?
Whether or not the function has a body is irrelevant here, I think. The `Def` here is just used for caching. So the first one makes sense, doesn't it?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136936/new/

https://reviews.llvm.org/D136936



More information about the cfe-commits mailing list