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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
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()))
----------------
tbaeder wrote:
> aaron.ballman wrote:
> > .... 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.
> I can see that going either way. I think your version is a more confusing though because the two body conditions are coupled with the `isDefined` condition. E.g. `HasBody` is `true` if `isDefined() && !willHaveBody()`, which doesn't make sense to me I think.
> 
> I think I just read this code too many times now.  How does being defined even relate to the function having a body? Should that code just be `HasBody = hasBody() || willHaveBody()`?
> I can see that going either way. I think your version is a more confusing though because the two body conditions are coupled with the isDefined condition. E.g. HasBody is true if isDefined() && !willHaveBody(), which doesn't make sense to me I think.

I'm glad we both are confused about the same thing!

> I think I just read this code too many times now. How does being defined even relate to the function having a body? Should that code just be HasBody = hasBody() || willHaveBody()?

I've not dug back in time to see how we got to that predicate in the first place, but my *hunch* is that this may have to do with implicit function definitions for things like special member functions. That's a case where you can have a function definition without a body. e.g.,
```
struct S {
  S();
};

S::S() = default;
```


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

https://reviews.llvm.org/D136936