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

[Paul Robinson via Phabricator via cfe-commits]

probinson added inline comments.


================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5596
+      DefaultedSMFArePOD = false;
+  }
+
----------------
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!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119051