[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

[David Blaikie via Phabricator via cfe-commits]

dblaikie added inline comments.


================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5596
+      DefaultedSMFArePOD = false;
+  }
+
----------------
probinson wrote:
> dblaikie wrote:
> > probinson wrote:
> > > dblaikie wrote:
> > > > probinson wrote:
> > > > > Does this mean `-fclang-abi-compat` will override the PS4/Darwin special case?  I think we don't want to do that.
> > > > I don't think so - this expression will make `DefaultedSMFArePOD` false when it's already false due to the target not being PS4 or Darwin, yeah? So it'll redundantly/harmlessly set it to false.
> > > No, I mean if it *is* PS4, it will turn true into false, and that's bad.  If I'm reading this correctly.
> > Right - let's see if I can explain how I'm understanding it at least - maybe I've got some code blindness from staring at it too long.
> > 
> > If it /is/ PS4, then this code:
> > ```
> > bool DefaultedSMFArePOD = !RawTriple.isPS4() && !RawTriple.isOSDarwin();
> > ```
> > Will amount to this:
> > ```
> > bool DefaultedSMFArePOD = !true && !RawTriple.isOSDarwin();
> > ```
> > ```
> > bool DefaultedSMFArePOD = false && !RawTriple.isOSDarwin();
> > ```
> > ```
> > bool DefaultedSMFArePOD = false;
> > ```
> > So then then the code at 5595 can only reinforce that, not change it - since it only sets the value to false. So my understanding is that the `-fclang-abi-compat` flag can not have any effect on the `DefaultedSMFArePOD` behavior of PS4. And it sounds like that's what you want? So I think we're good here?
> Those little bangs can be hard to see sometimes...  Also, I think this bit
> > already false due to the target not being PS4 or Darwin, 
> confused me, should have read
> > already false due to the target being PS4 or Darwin,
> and so what the code actually does is in fact what we want.
> 
> Thanks for your patience in explaining it, and I will go make an eye doctor appointment!
>  I think this bit
> > already false due to the target not being PS4 or Darwin,
> confused me, should have read
> > already false due to the target being PS4 or Darwin,

Ah, right you are, sorry about that!

> Thanks for your patience in explaining it, and I will go make an eye doctor appointment!

Sure thing - certainly sometimes consider whether or not switching to the C++ alternative tokens would be an improvement...
```
DefaultedSMFArePOD = not RawTriple.isPS4() and not RawTriple.isOSDarwin()
```
 ;)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119051