[PATCH] D50531: [NFC] Convert ParsedAttr to use llvm::TrailingObjects

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.


================
Comment at: include/clang/Sema/ParsedAttr.h:80
 
+struct TypeTagForDatatypeData {
+  ParsedType *MatchingCType;
----------------
Because all of these became part of the TYPE of ParsedAttr now, they need to be defined outside of ParsedAttr.  Additionally, they are now required to be in a non-anonymous namespace.


================
Comment at: include/clang/Sema/ParsedAttr.h:525
 
-  const PropertyData &getPropertyData() const {
-    assert(isDeclspecPropertyAttribute() && "Not a __delcspec(property) attribute");
-    return getPropertyDataBuffer();
+  IdentifierInfo *getPropertyDataGetter() const {
+    assert(isDeclspecPropertyAttribute() &&
----------------
Unfortunately, these need to return a non-const param, since the only user of these  takes a const ParsedAttr and requires the IdentifierInfo* to be a non-const ptr.


================
Comment at: include/clang/Sema/ParsedAttr.h:578
     AvailabilityAllocSize =
-        sizeof(ParsedAttr) +
-        ((sizeof(AvailabilityData) + sizeof(void *) + sizeof(ArgsUnion) - 1) /
-         sizeof(void *) * sizeof(void *)),
-    TypeTagForDatatypeAllocSize = sizeof(ParsedAttr) +
-                                  (sizeof(ParsedAttr::TypeTagForDatatypeData) +
-                                   sizeof(void *) + sizeof(ArgsUnion) - 1) /
-                                      sizeof(void *) * sizeof(void *),
+        ParsedAttr::totalSizeToAlloc<ArgsUnion, detail::AvailabilityData,
+                                     detail::TypeTagForDatatypeData, ParsedType,
----------------
Sadly, there isn't a better way to do this with TrailingObjects.  The parameter argument list is simply so that it can check to make sure you have them right?

I'd much prefer to be able to omit the ones not important to this size, but that doesn't seem possible.

Also, it seems that we used to do some things to make sure that these sizes were a multiple of sizeof(void*) as a premature optimization.  However, all of these are constant-sized, so over-allocating seems like a useless task.


================
Comment at: lib/Sema/SemaDeclCXX.cpp:15466
   SourceLocation TSSL = D.getLocStart();
-  const ParsedAttr::PropertyData &Data = MSPropertyAttr.getPropertyData();
-  MSPropertyDecl *NewPD = MSPropertyDecl::Create(
-      Context, Record, Loc, II, T, TInfo, TSSL, Data.GetterId, Data.SetterId);
+  MSPropertyDecl *NewPD =
+      MSPropertyDecl::Create(Context, Record, Loc, II, T, TInfo, TSSL,
----------------
The only usage of PropertyData, which didn't seem important to support anymore.


https://reviews.llvm.org/D50531