[NFC] Convert ParsedAttr to use llvm::TrailingObjects

[erichkeane]

Summary

ParsedAttr is using a hand-rolled trailing-objects
implementation that gets cleaned up quite a bit by
just using llvm::TrailingObjects. This is a large
TrailingObjects list, but most things are length '0'.

[erichkeane]

include/clang/Sema/ParsedAttr.h

80 ↗

(On Diff #159980)

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.

525 ↗

(On Diff #159980)

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.

578 ↗

(On Diff #159980)

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.

lib/Sema/SemaDeclCXX.cpp

15466 ↗

(On Diff #159980)

The only usage of PropertyData, which didn't seem important to support anymore.

[rsmith]

Looks good, thanks. (I'm not sure why PropertyData needs special handling rather than being treated as two IdentifierLoc arguments, but that's not made any worse here, just perhaps a bit more obvious.)

[riccibruno]

Some inline comments. Since you already committed it I don't think it
is worth bothering with them.

cfe/trunk/include/clang/Sema/ParsedAttr.h

124

I think you can do something like friend TrailingObjects;
like most (every ?) other users of TrailingObjects do. It relies on the
injected class name if I am not mistaken.

366

And maybe it would be nice to delete explicitly the move
ctor/assignment op too (my turn to make this remark :) ).
They are already not declared I think so it do not really matter.

cfe/trunk/include/clang/Sema/ParsedAttr.h

struct AvailabilityChange {
 
   /// The source range covering the version number.
   SourceRange VersionRange;
 
   /// Determine whether this availability change is valid.
   bool isValid() const { return !Version.empty(); }
 };
 
-namespace {
-
+namespace detail {
 enum AvailabilitySlot {
   IntroducedSlot, DeprecatedSlot, ObsoletedSlot, NumAvailabilitySlots
 };
 
 /// Describes the trailing object for Availability attribute in ParsedAttr.
 struct AvailabilityData {
   AvailabilityChange Changes[NumAvailabilitySlots];
   SourceLocation StrictLoc;
   const Expr *Replacement;
 
   AvailabilityData(const AvailabilityChange &Introduced,
                    const AvailabilityChange &Deprecated,
                    const AvailabilityChange &Obsoleted,
                    SourceLocation Strict, const Expr *ReplaceExpr)
     : StrictLoc(Strict), Replacement(ReplaceExpr) {
     Changes[IntroducedSlot] = Introduced;
     Changes[DeprecatedSlot] = Deprecated;
     Changes[ObsoletedSlot] = Obsoleted;
   }
 };
 
+struct TypeTagForDatatypeData {
+  ParsedType *MatchingCType;
+  unsigned LayoutCompatible : 1;
+  unsigned MustBeNull : 1;
+};
+struct PropertyData {
+  IdentifierInfo *GetterId, *SetterId;
+
+  PropertyData(IdentifierInfo *getterId, IdentifierInfo *setterId)
+      : GetterId(getterId), SetterId(setterId) {}
+};
+
 } // namespace
 
 /// Wraps an identifier and optional source location for the identifier.
 struct IdentifierLoc {
   SourceLocation Loc;
   IdentifierInfo *Ident;
 
   static IdentifierLoc *create(ASTContext &Ctx, SourceLocation Loc,

 ///
 /// For a GNU attribute, there are four forms of this construct:
 ///
 /// 1: __attribute__(( const )). ParmName/Args/NumArgs will all be unused.
 /// 2: __attribute__(( mode(byte) )). ParmName used, Args/NumArgs unused.
 /// 3: __attribute__(( format(printf, 1, 2) )). ParmName/Args/NumArgs all used.
 /// 4: __attribute__(( aligned(16) )). ParmName is unused, Args/Num used.
 ///
-class ParsedAttr {
+class ParsedAttr final
+    : private llvm::TrailingObjects<
+          ParsedAttr, ArgsUnion, detail::AvailabilityData,
+          detail::TypeTagForDatatypeData, ParsedType, detail::PropertyData> {
+  friend class llvm::TrailingObjects<
+      ParsedAttr, ArgsUnion, detail::AvailabilityData,
+      detail::TypeTagForDatatypeData, ParsedType, detail::PropertyData>;
+
+  size_t numTrailingObjects(OverloadToken<ArgsUnion>) const { return NumArgs; }
+  size_t numTrailingObjects(OverloadToken<detail::AvailabilityData>) const {
+    return IsAvailability;
+  }
+  size_t
+      numTrailingObjects(OverloadToken<detail::TypeTagForDatatypeData>) const {
+    return IsTypeTagForDatatype;
+  }
+  size_t numTrailingObjects(OverloadToken<ParsedType>) const {
+    return HasParsedType;
+  }
+  size_t numTrailingObjects(OverloadToken<detail::PropertyData>) const {
+    return IsProperty;
+  }
+
 public:
   /// The style used to specify an attribute.
   enum Syntax {
     /// __attribute__((...))
     AS_GNU,
 
     /// [[...]]
     AS_CXX11,
private:
   mutable unsigned ProcessingCache : 8;
 
   /// The location of the 'unavailable' keyword in an
   /// availability attribute.
   SourceLocation UnavailableLoc;
 
   const Expr *MessageExpr;
 
-  /// Arguments, if any, are stored immediately following the object.
-  ArgsUnion *getArgsBuffer() { return reinterpret_cast<ArgsUnion *>(this + 1); }
+  ArgsUnion *getArgsBuffer() { return getTrailingObjects<ArgsUnion>(); }
   ArgsUnion const *getArgsBuffer() const {
-    return reinterpret_cast<ArgsUnion const *>(this + 1);
+    return getTrailingObjects<ArgsUnion>();
   }
 
-  /// Availability information is stored immediately following the arguments,
-  /// if any, at the end of the object.
-  AvailabilityData *getAvailabilityData() {
-    return reinterpret_cast<AvailabilityData*>(getArgsBuffer() + NumArgs);
+  detail::AvailabilityData *getAvailabilityData() {
+    return getTrailingObjects<detail::AvailabilityData>();
   }
-  const AvailabilityData *getAvailabilityData() const {
-    return reinterpret_cast<const AvailabilityData*>(getArgsBuffer() + NumArgs);
+  const detail::AvailabilityData *getAvailabilityData() const {
+    return getTrailingObjects<detail::AvailabilityData>();
   }
 
-public:
-  struct TypeTagForDatatypeData {
-    ParsedType *MatchingCType;
-    unsigned LayoutCompatible : 1;
-    unsigned MustBeNull : 1;
-  };
-  struct PropertyData {
-    IdentifierInfo *GetterId, *SetterId;
-
-    PropertyData(IdentifierInfo *getterId, IdentifierInfo *setterId)
-        : GetterId(getterId), SetterId(setterId) {}
-  };
-
 private:
   friend class AttributeFactory;
   friend class AttributePool;
 
   /// Constructor for attributes with expression arguments.
   ParsedAttr(IdentifierInfo *attrName, SourceRange attrRange,
              IdentifierInfo *scopeName, SourceLocation scopeLoc,
              ArgsUnion *args, unsigned numArgs, Syntax syntaxUsed,
ParsedAttr(IdentifierInfo *attrName, SourceRange attrRange,
       : AttrName(attrName), ScopeName(scopeName), AttrRange(attrRange),
         ScopeLoc(scopeLoc), NumArgs(1), SyntaxUsed(syntaxUsed), Invalid(false),
         UsedAsTypeAttr(false), IsAvailability(true),
         IsTypeTagForDatatype(false), IsProperty(false), HasParsedType(false),
         HasProcessingCache(false), UnavailableLoc(unavailable),
         MessageExpr(messageExpr) {
     ArgsUnion PVal(Parm);
     memcpy(getArgsBuffer(), &PVal, sizeof(ArgsUnion));
-    new (getAvailabilityData()) AvailabilityData(
+    new (getAvailabilityData()) detail::AvailabilityData(
         introduced, deprecated, obsoleted, strict, replacementExpr);
     AttrKind = getKind(getName(), getScopeName(), syntaxUsed);
   }
 
   /// Constructor for objc_bridge_related attributes.
   ParsedAttr(IdentifierInfo *attrName, SourceRange attrRange,
              IdentifierInfo *scopeName, SourceLocation scopeLoc,
              IdentifierLoc *Parm1, IdentifierLoc *Parm2, IdentifierLoc *Parm3,
ParsedAttr(IdentifierInfo *attrName, SourceRange attrRange,
              bool layoutCompatible, bool mustBeNull, Syntax syntaxUsed)
       : AttrName(attrName), ScopeName(scopeName), AttrRange(attrRange),
         ScopeLoc(scopeLoc), NumArgs(1), SyntaxUsed(syntaxUsed), Invalid(false),
         UsedAsTypeAttr(false), IsAvailability(false),
         IsTypeTagForDatatype(true), IsProperty(false), HasParsedType(false),
         HasProcessingCache(false) {
     ArgsUnion PVal(ArgKind);
     memcpy(getArgsBuffer(), &PVal, sizeof(ArgsUnion));
-    TypeTagForDatatypeData &ExtraData = getTypeTagForDatatypeDataSlot();
+    detail::TypeTagForDatatypeData &ExtraData = getTypeTagForDatatypeDataSlot();
     new (&ExtraData.MatchingCType) ParsedType(matchingCType);
     ExtraData.LayoutCompatible = layoutCompatible;
     ExtraData.MustBeNull = mustBeNull;
     AttrKind = getKind(getName(), getScopeName(), syntaxUsed);
   }
 
   /// Constructor for attributes with a single type argument.
   ParsedAttr(IdentifierInfo *attrName, SourceRange attrRange,
ParsedAttr(IdentifierInfo *attrName, SourceRange attrRange,
              IdentifierInfo *scopeName, SourceLocation scopeLoc,
              IdentifierInfo *getterId, IdentifierInfo *setterId,
              Syntax syntaxUsed)
       : AttrName(attrName), ScopeName(scopeName), AttrRange(attrRange),
         ScopeLoc(scopeLoc), NumArgs(0), SyntaxUsed(syntaxUsed), Invalid(false),
         UsedAsTypeAttr(false), IsAvailability(false),
         IsTypeTagForDatatype(false), IsProperty(true), HasParsedType(false),
         HasProcessingCache(false) {
-    new (&getPropertyDataBuffer()) PropertyData(getterId, setterId);
+    new (&getPropertyDataBuffer()) detail::PropertyData(getterId, setterId);
     AttrKind = getKind(getName(), getScopeName(), syntaxUsed);
   }
 
   /// Type tag information is stored immediately following the arguments, if
   /// any, at the end of the object.  They are mutually exclusive with
   /// availability slots.
-  TypeTagForDatatypeData &getTypeTagForDatatypeDataSlot() {
-    return *reinterpret_cast<TypeTagForDatatypeData*>(getArgsBuffer()+NumArgs);
+  detail::TypeTagForDatatypeData &getTypeTagForDatatypeDataSlot() {
+    return *getTrailingObjects<detail::TypeTagForDatatypeData>();
   }
-  const TypeTagForDatatypeData &getTypeTagForDatatypeDataSlot() const {
-    return *reinterpret_cast<const TypeTagForDatatypeData*>(getArgsBuffer()
-                                                            + NumArgs);
+  const detail::TypeTagForDatatypeData &getTypeTagForDatatypeDataSlot() const {
+    return *getTrailingObjects<detail::TypeTagForDatatypeData>();
   }
 
   /// The type buffer immediately follows the object and are mutually exclusive
   /// with arguments.
-  ParsedType &getTypeBuffer() {
-    return *reinterpret_cast<ParsedType *>(this + 1);
-  }
+  ParsedType &getTypeBuffer() { return *getTrailingObjects<ParsedType>(); }
   const ParsedType &getTypeBuffer() const {
-    return *reinterpret_cast<const ParsedType *>(this + 1);
+    return *getTrailingObjects<ParsedType>();
   }
 
   /// The property data immediately follows the object is is mutually exclusive
   /// with arguments.
-  PropertyData &getPropertyDataBuffer() {
+  detail::PropertyData &getPropertyDataBuffer() {
     assert(IsProperty);
-    return *reinterpret_cast<PropertyData*>(this + 1);
+    return *getTrailingObjects<detail::PropertyData>();
   }
-  const PropertyData &getPropertyDataBuffer() const {
+  const detail::PropertyData &getPropertyDataBuffer() const {
     assert(IsProperty);
-    return *reinterpret_cast<const PropertyData*>(this + 1);
+    return *getTrailingObjects<detail::PropertyData>();
   }
 
   size_t allocated_size() const;
 
 public:
   ParsedAttr(const ParsedAttr &) = delete;
   ParsedAttr &operator=(const ParsedAttr &) = delete;
   ~ParsedAttr() = delete;
 
   void operator delete(void *) = delete;
 
   enum Kind {
     #define PARSED_ATTR(NAME) AT_##NAME,
     #include "clang/Sema/AttrParsedAttrList.inc"
     #undef PARSED_ATTR
     IgnoredAttribute,
public:
   }
 
   IdentifierLoc *getArgAsIdent(unsigned Arg) const {
     return getArg(Arg).get<IdentifierLoc*>();
   }
 
   const AvailabilityChange &getAvailabilityIntroduced() const {
     assert(getKind() == AT_Availability && "Not an availability attribute");
-    return getAvailabilityData()->Changes[IntroducedSlot];
+    return getAvailabilityData()->Changes[detail::IntroducedSlot];
   }
 
   const AvailabilityChange &getAvailabilityDeprecated() const {
     assert(getKind() == AT_Availability && "Not an availability attribute");
-    return getAvailabilityData()->Changes[DeprecatedSlot];
+    return getAvailabilityData()->Changes[detail::DeprecatedSlot];
   }
 
   const AvailabilityChange &getAvailabilityObsoleted() const {
     assert(getKind() == AT_Availability && "Not an availability attribute");
-    return getAvailabilityData()->Changes[ObsoletedSlot];
+    return getAvailabilityData()->Changes[detail::ObsoletedSlot];
   }
 
   SourceLocation getStrictLoc() const {
     assert(getKind() == AT_Availability && "Not an availability attribute");
     return getAvailabilityData()->StrictLoc;
   }
 
   SourceLocation getUnavailableLoc() const {
bool getMustBeNull() const {
     return getTypeTagForDatatypeDataSlot().MustBeNull;
   }
 
   const ParsedType &getTypeArg() const {
     assert(HasParsedType && "Not a type attribute");
     return getTypeBuffer();
   }
 
-  const PropertyData &getPropertyData() const {
-    assert(isDeclspecPropertyAttribute() && "Not a __delcspec(property) attribute");
-    return getPropertyDataBuffer();
+  IdentifierInfo *getPropertyDataGetter() const {
+    assert(isDeclspecPropertyAttribute() &&
+           "Not a __delcspec(property) attribute");
+    return getPropertyDataBuffer().GetterId;
+  }
+
+  IdentifierInfo *getPropertyDataSetter() const {
+    assert(isDeclspecPropertyAttribute() &&
+           "Not a __delcspec(property) attribute");
+    return getPropertyDataBuffer().SetterId;
   }
 
   /// Get an index into the attribute spelling list
   /// defined in Attr.td. This index is used by an attribute
   /// to pretty print itself.
   unsigned getAttributeSpellingListIndex() const;
 
   bool isTargetSpecificAttr() const;

 /// A factory, from which one makes pools, from which one creates
 /// individual attributes which are deallocated with the pool.
 ///
 /// Note that it's tolerably cheap to create and destroy one of
 /// these as long as you don't actually allocate anything in it.
 class AttributeFactory {
 public:
   enum {
-    /// The required allocation size of an availability attribute,
-    /// which we want to ensure is a multiple of sizeof(void*).
     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,
+                                     detail::PropertyData>(1, 1, 0, 0, 0),
+    TypeTagForDatatypeAllocSize =
+        ParsedAttr::totalSizeToAlloc<ArgsUnion, detail::AvailabilityData,
+                                     detail::TypeTagForDatatypeData, ParsedType,
+                                     detail::PropertyData>(0, 0, 1, 0, 0),
     PropertyAllocSize =
-        sizeof(ParsedAttr) +
-        (sizeof(ParsedAttr::PropertyData) + sizeof(void *) - 1) /
-            sizeof(void *) * sizeof(void *)
+        ParsedAttr::totalSizeToAlloc<ArgsUnion, detail::AvailabilityData,
+                                     detail::TypeTagForDatatypeData, ParsedType,
+                                     detail::PropertyData>(0, 0, 0, 0, 1),
   };
 
 private:
   enum {
     /// The number of free lists we want to be sure to support
     /// inline.  This is just enough that availability attributes
     /// don't surpass it.  It's actually very unlikely we'll see an
     /// attribute that needs more than that; on x86-64 you'd need 10
void takeAllFrom(AttributePool &pool) {
     pool.Attrs.clear();
   }
 
   ParsedAttr *create(IdentifierInfo *attrName, SourceRange attrRange,
                      IdentifierInfo *scopeName, SourceLocation scopeLoc,
                      ArgsUnion *args, unsigned numArgs,
                      ParsedAttr::Syntax syntax,
                      SourceLocation ellipsisLoc = SourceLocation()) {
-    void *memory = allocate(sizeof(ParsedAttr) + numArgs * sizeof(ArgsUnion));
+    size_t temp =
+        ParsedAttr::totalSizeToAlloc<ArgsUnion, detail::AvailabilityData,
+                                     detail::TypeTagForDatatypeData, ParsedType,
+                                     detail::PropertyData>(numArgs, 0, 0, 0, 0);
+    (void)temp;
+    void *memory = allocate(
+        ParsedAttr::totalSizeToAlloc<ArgsUnion, detail::AvailabilityData,
+                                     detail::TypeTagForDatatypeData, ParsedType,
+                                     detail::PropertyData>(numArgs, 0, 0, 0,
+                                                           0));
     return add(new (memory) ParsedAttr(attrName, attrRange, scopeName, scopeLoc,
                                        args, numArgs, syntax, ellipsisLoc));
   }
 
   ParsedAttr *create(IdentifierInfo *attrName, SourceRange attrRange,
                      IdentifierInfo *scopeName, SourceLocation scopeLoc,
                      IdentifierLoc *Param, const AvailabilityChange &introduced,
                      const AvailabilityChange &deprecated,
                      const AvailabilityChange &obsoleted,
                      SourceLocation unavailable, const Expr *MessageExpr,
                      ParsedAttr::Syntax syntax, SourceLocation strict,
                      const Expr *ReplacementExpr) {
     void *memory = allocate(AttributeFactory::AvailabilityAllocSize);
     return add(new (memory) ParsedAttr(
         attrName, attrRange, scopeName, scopeLoc, Param, introduced, deprecated,
         obsoleted, unavailable, MessageExpr, syntax, strict, ReplacementExpr));
   }
 
   ParsedAttr *create(IdentifierInfo *attrName, SourceRange attrRange,
                      IdentifierInfo *scopeName, SourceLocation scopeLoc,
                      IdentifierLoc *Param1, IdentifierLoc *Param2,
                      IdentifierLoc *Param3, ParsedAttr::Syntax syntax) {
-    size_t size = sizeof(ParsedAttr) + 3 * sizeof(ArgsUnion);
-    void *memory = allocate(size);
+    void *memory = allocate(
+        ParsedAttr::totalSizeToAlloc<ArgsUnion, detail::AvailabilityData,
+                                     detail::TypeTagForDatatypeData, ParsedType,
+                                     detail::PropertyData>(3, 0, 0, 0, 0));
     return add(new (memory) ParsedAttr(attrName, attrRange, scopeName, scopeLoc,
                                        Param1, Param2, Param3, syntax));
   }
 
   ParsedAttr *
   createTypeTagForDatatype(IdentifierInfo *attrName, SourceRange attrRange,
                            IdentifierInfo *scopeName, SourceLocation scopeLoc,
                            IdentifierLoc *argumentKind,
                            ParsedType matchingCType, bool layoutCompatible,
                            bool mustBeNull, ParsedAttr::Syntax syntax) {
     void *memory = allocate(AttributeFactory::TypeTagForDatatypeAllocSize);
     return add(new (memory) ParsedAttr(attrName, attrRange, scopeName, scopeLoc,
                                        argumentKind, matchingCType,
                                        layoutCompatible, mustBeNull, syntax));
   }
 
   ParsedAttr *createTypeAttribute(IdentifierInfo *attrName,
                                   SourceRange attrRange,
                                   IdentifierInfo *scopeName,
                                   SourceLocation scopeLoc, ParsedType typeArg,
                                   ParsedAttr::Syntax syntaxUsed) {
-    void *memory = allocate(sizeof(ParsedAttr) + sizeof(void *));
+    void *memory = allocate(
+        ParsedAttr::totalSizeToAlloc<ArgsUnion, detail::AvailabilityData,
+                                     detail::TypeTagForDatatypeData, ParsedType,
+                                     detail::PropertyData>(0, 0, 0, 1, 0));
     return add(new (memory) ParsedAttr(attrName, attrRange, scopeName, scopeLoc,
                                        typeArg, syntaxUsed));
   }
 
   ParsedAttr *
   createPropertyAttribute(IdentifierInfo *attrName, SourceRange attrRange,
                           IdentifierInfo *scopeName, SourceLocation scopeLoc,
                           IdentifierInfo *getterId, IdentifierInfo *setterId,

cfe/trunk/lib/Sema/ParsedAttr.cpp

 
 size_t ParsedAttr::allocated_size() const {
   if (IsAvailability) return AttributeFactory::AvailabilityAllocSize;
   else if (IsTypeTagForDatatype)
     return AttributeFactory::TypeTagForDatatypeAllocSize;
   else if (IsProperty)
     return AttributeFactory::PropertyAllocSize;
   else if (HasParsedType)
-    return sizeof(ParsedAttr) + sizeof(void *);
-  return (sizeof(ParsedAttr) + NumArgs * sizeof(ArgsUnion));
+    return totalSizeToAlloc<ArgsUnion, detail::AvailabilityData,
+                            detail::TypeTagForDatatypeData, ParsedType,
+                            detail::PropertyData>(0, 0, 0, 1, 0);
+  return totalSizeToAlloc<ArgsUnion, detail::AvailabilityData,
+                          detail::TypeTagForDatatypeData, ParsedType,
+                          detail::PropertyData>(NumArgs, 0, 0, 0, 0);
 }
 
 AttributeFactory::AttributeFactory() {
   // Go ahead and configure all the inline capacity.  This is just a memset.
   FreeLists.resize(InlineFreeListsCapacity);
 }
 AttributeFactory::~AttributeFactory() = default;
 

cfe/trunk/lib/Sema/SemaDeclCXX.cpp

if (PrevDecl && PrevDecl->isTemplateParameter()) {
     // Just pretend that we didn't see the previous declaration.
     PrevDecl = nullptr;
   }
 
   if (PrevDecl && !isDeclInScope(PrevDecl, Record, S))
     PrevDecl = nullptr;
 
   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,
+                             MSPropertyAttr.getPropertyDataGetter(),
+                             MSPropertyAttr.getPropertyDataSetter());
   ProcessDeclAttributes(TUScope, NewPD, D);
   NewPD->setAccess(AS);
 
   if (NewPD->isInvalidDecl())
     Record->setInvalidDecl();
 
   if (D.getDeclSpec().isModulePrivateSpecified())
     NewPD->setModulePrivate();