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:
> > 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;
> > ```
> Well, any sort of digging usually just gets you to the //one// commit that 
> introduced all of the new constant interpreter :P 
> 
> `willHaveBody()` isn't even relevant here, is it?
> I think simply
> 
> ``` 
> bool hasBody = FuncDecl->isDefined() || FuncDecl->hasBody();
> 
> ```
> makes the most sense.
I think it's one or the other?

`isDefined` searches for a function that's been defined by looking for 
something in the redecl chain that: is deleted as written, is defaulted, has a 
body, will have a body, has a body that's skipped, or has an attribute that 
causes the declaration to be a definition. 

By contrast, `hasBody` searches the redecl chain looking for a declaration that 
has not been defaulted and has a body or is template late parsed.

So I *think* what you want is `FuncDecl->hasBody(FuncDecl)` to do the check and 
get the specific declaration with the body.


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

https://reviews.llvm.org/D136936

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D136936: [clang][Int... Aaron Ballman via Phabricator via cfe-commits

Reply via email to