[PATCH] D31513: [Sema] Add __is_aggregate type-trait and implement LWG 2015

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: docs/LanguageExtensions.rst:996
 * ``__is_abstract`` (GNU, Microsoft)
+* ``__is_aggregate`` (GNU, Microsoft)
 * ``__is_base_of`` (GNU, Microsoft)
----------------
EricWF wrote:
> aaron.ballman wrote:
> > Has Microsoft already implemented this? If not, do we want to wait for them before claiming they implement it as well?
> I asked @STL_MSFT to ping the frontend team to confirm they were planning on implementing it with this name. I was concerned this doc would never get updated otherwise. 
Seems reasonable. My concern is that we claim Microsoft implements this when they don't (yet) and someone uses this documentation to try to force Microsoft's hand. However, it seems that this documentation not being updated is the far more likely scenario. ;-)


================
Comment at: lib/Sema/SemaExprCXX.cpp:4089
+    // See LWG 2015
+    QualType ElTy = S.Context.getBaseElementType(ArgTy);
     if (ElTy->isVoidType())
----------------
If I understand properly, this change is required by LWG 2015 for existing type traits *and* is needed for is_aggregate()? If so, it probably should be a separate patch doing just LWG 2015 (and tests) and a second one for is_aggregate().


================
Comment at: lib/Sema/SemaExprCXX.cpp:4234
+    // support aggregate initialization. GCC mirrors this behavior for vectors
+    // but not _Complex.
+    return T->isAggregateType() || T->isVectorType() || T->isExtVectorType() ||
----------------
EricWF wrote:
> aaron.ballman wrote:
> > Is there benefit to diverging from GCC's behavior here for _Complex types?
> `_Complex` types act as if they are an array of 2 elements, and hence support aggregate initialization.
> 
> Ironically the GCC maintainer who implemented `is_aggregate` pointed this case out to me. I've pinged him to ask why he chose not to support it, or if the change is in the works.
Thanks! I think supporting it makes sense.


https://reviews.llvm.org/D31513