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

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Nov 3 11:46:37 PDT 2022


aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM!



================
Comment at: clang/lib/AST/Interp/ByteCodeEmitter.cpp:24-31
+  bool HasBody = true;
+
+  // Function is not defined at all or not yet. We will
+  // create a Function instance but not compile the body. That
+  // will (maybe) happen later.
   if (!FuncDecl->isDefined(FuncDecl) ||
       (!FuncDecl->hasBody() && FuncDecl->willHaveBody()))
----------------
.... negating the Boolean calculation and applying deMorgan's law did not make that code more clear, did it (assuming I did everything right)? If you agree, then I'm fine with the more complicated form and letting the optimizer make it faster.


================
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)...);
----------------
tbaeder wrote:
> 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?
Hmmm, this may actually be fine. The situation I had in mind is where a redeclaration adds more semantic information to the function declaration that may disqualify it from being a constexpr function due to UB. For example, declare a function with no attribute, then redeclare the function with an attribute accepting an expression argument (like `__attribute__((enable_if))` or `__attribute__((diagnose_if))` and put the UB in the expression argument.

But I can't devise a test case that hits the problem because any *use* of the `constexpr` function causes us to claim it's non-constexpr if the function is declared but not defined.


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

https://reviews.llvm.org/D136936



More information about the cfe-commits mailing list