<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, May 23, 2016 at 10:28 AM, Aaron Ballman <span dir="ltr"><<a href="mailto:aaron@aaronballman.com" target="_blank">aaron@aaronballman.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div><div>On Mon, May 23, 2016 at 1:16 PM, David Majnemer via cfe-commits<br>
<<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>> wrote:<br>
> Author: majnemer<br>
> Date: Mon May 23 12:16:12 2016<br>
> New Revision: 270457<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=270457&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=270457&view=rev</a><br>
> Log:<br>
> [MS ABI] Implement __declspec(empty_bases) and __declspec(layout_version)<br>
><br>
> The layout_version attribute is pretty straightforward: use the layout<br>
> rules from version XYZ of MSVC when used like<br>
> struct __declspec(layout_version(XYZ)) S {};<br>
><br>
> The empty_bases attribute is more interesting. It tries to get the C++<br>
> empty base optimization to fire more often by tweaking the MSVC ABI<br>
> rules in subtle ways:<br>
> 1. Disable the leading and trailing zero-sized object flags if a class<br>
> is marked __declspec(empty_bases) and is empty.<br>
><br>
> This means that given:<br>
> struct __declspec(empty_bases) A {};<br>
> struct __declspec(empty_bases) B {};<br>
> struct C : A, B {};<br>
><br>
> 'C' will have size 1 and nvsize 0 despite not being annotated<br>
> __declspec(empty_bases).<br>
><br>
> 2. When laying out virtual or non-virtual bases, disable the injection<br>
> of padding between classes if the most derived class is marked<br>
> __declspec(empty_bases).<br>
><br>
> This means that given:<br>
> struct A {};<br>
> struct B {};<br>
> struct __declspec(empty_bases) C : A, B {};<br>
><br>
> 'C' will have size 1 and nvsize 0.<br>
><br>
> 3. When calculating the offset of a non-virtual base, choose offset zero<br>
> if the most derived class is marked __declspec(empty_bases) and the<br>
> base is empty _and_ has an nvsize of 0.<br>
><br>
> Because of the ABI rules, this does not mean that empty bases<br>
> reliably get placed at offset 0!<br>
><br>
> For example:<br>
> struct A {};<br>
> struct B {};<br>
> struct __declspec(empty_bases) C : A, B { virtual ~C(); };<br>
><br>
> 'C' will be pointer sized to account for the vfptr at offset 0.<br>
> 'A' and 'B' will _not_ be at offset 0 despite being empty!<br>
> Instead, they will be located right after the vfptr.<br>
><br>
> This occurs due to the interaction betweeen non-virtual base layout<br>
> and virtual function pointer injection: injection occurs after the<br>
> nv-bases and shifts them down by the size of a pointer.<br>
><br>
> Added:<br>
> cfe/trunk/test/Layout/ms-x86-declspec-empty_bases.cpp<br>
> cfe/trunk/test/SemaCXX/ms-empty_bases.cpp<br>
> cfe/trunk/test/SemaCXX/ms-layout_version.cpp<br>
> Modified:<br>
> cfe/trunk/include/clang/AST/RecordLayout.h<br>
> cfe/trunk/include/clang/Basic/Attr.td<br>
> cfe/trunk/include/clang/Basic/AttrDocs.td<br>
> cfe/trunk/lib/AST/RecordLayout.cpp<br>
> cfe/trunk/lib/AST/RecordLayoutBuilder.cpp<br>
> cfe/trunk/lib/Sema/SemaDeclAttr.cpp<br>
><br>
> Modified: cfe/trunk/include/clang/AST/RecordLayout.h<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/RecordLayout.h?rev=270457&r1=270456&r2=270457&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/RecordLayout.h?rev=270457&r1=270456&r2=270457&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/include/clang/AST/RecordLayout.h (original)<br>
> +++ cfe/trunk/include/clang/AST/RecordLayout.h Mon May 23 12:16:12 2016<br>
> @@ -104,10 +104,10 @@ private:<br>
> /// a primary base class.<br>
> bool HasExtendableVFPtr : 1;<br>
><br>
> - /// HasZeroSizedSubObject - True if this class contains a zero sized member<br>
> - /// or base or a base with a zero sized member or base. Only used for<br>
> - /// MS-ABI.<br>
> - bool HasZeroSizedSubObject : 1;<br>
> + /// EndsWithZeroSizedObject - True if this class contains a zero sized<br>
> + /// member or base or a base with a zero sized member or base.<br>
> + /// Only used for MS-ABI.<br>
> + bool EndsWithZeroSizedObject : 1;<br>
><br>
> /// \brief True if this class is zero sized or first base is zero sized or<br>
> /// has this property. Only used for MS-ABI.<br>
> @@ -154,7 +154,7 @@ private:<br>
> const CXXRecordDecl *PrimaryBase,<br>
> bool IsPrimaryBaseVirtual,<br>
> const CXXRecordDecl *BaseSharingVBPtr,<br>
> - bool HasZeroSizedSubObject,<br>
> + bool EndsWithZeroSizedObject,<br>
> bool LeadsWithZeroSizedBase,<br>
> const BaseOffsetsMapTy& BaseOffsets,<br>
> const VBaseOffsetsMapTy& VBaseOffsets);<br>
> @@ -283,8 +283,8 @@ public:<br>
> return RequiredAlignment;<br>
> }<br>
><br>
> - bool hasZeroSizedSubObject() const {<br>
> - return CXXInfo && CXXInfo->HasZeroSizedSubObject;<br>
> + bool endsWithZeroSizedObject() const {<br>
> + return CXXInfo && CXXInfo->EndsWithZeroSizedObject;<br>
> }<br>
><br>
> bool leadsWithZeroSizedBase() const {<br>
><br>
> Modified: cfe/trunk/include/clang/Basic/Attr.td<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=270457&r1=270456&r2=270457&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=270457&r1=270456&r2=270457&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/include/clang/Basic/Attr.td (original)<br>
> +++ cfe/trunk/include/clang/Basic/Attr.td Mon May 23 12:16:12 2016<br>
> @@ -745,6 +745,12 @@ def Destructor : InheritableAttr {<br>
> let Documentation = [Undocumented];<br>
> }<br>
><br>
> +def EmptyBases : InheritableAttr, TargetSpecificAttr<TargetMicrosoftCXXABI> {<br>
> + let Spellings = [Declspec<"empty_bases">];<br>
> + let Subjects = SubjectList<[CXXRecord]>;<br>
> + let Documentation = [EmptyBasesDocs];<br>
> +}<br>
> +<br>
> def EnableIf : InheritableAttr {<br>
> let Spellings = [GNU<"enable_if">];<br>
> let Subjects = SubjectList<[Function]>;<br>
> @@ -868,6 +874,13 @@ def Restrict : InheritableAttr {<br>
> let Documentation = [Undocumented];<br>
> }<br>
><br>
> +def LayoutVersion : InheritableAttr, TargetSpecificAttr<TargetMicrosoftCXXABI> {<br>
> + let Spellings = [Declspec<"layout_version">];<br>
> + let Args = [UnsignedArgument<"Version">];<br>
> + let Subjects = SubjectList<[CXXRecord]>;<br>
> + let Documentation = [LayoutVersionDocs];<br>
> +}<br>
> +<br>
> def MaxFieldAlignment : InheritableAttr {<br>
> // This attribute has no spellings as it is only ever created implicitly.<br>
> let Spellings = [];<br>
><br>
> Modified: cfe/trunk/include/clang/Basic/AttrDocs.td<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/AttrDocs.td?rev=270457&r1=270456&r2=270457&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/AttrDocs.td?rev=270457&r1=270456&r2=270457&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/include/clang/Basic/AttrDocs.td (original)<br>
> +++ cfe/trunk/include/clang/Basic/AttrDocs.td Mon May 23 12:16:12 2016<br>
> @@ -1553,6 +1553,24 @@ manipulating bits of the enumerator when<br>
> }];<br>
> }<br>
><br>
> +def EmptyBasesDocs : Documentation {<br>
> + let Category = DocCatType;<br>
> + let Content = [{<br>
> +The empty_bases attribute permits the compiler to utilize the<br>
> +empty-base-optimization more frequently.<br>
> +It is only supported when using the Microsoft C++ ABI.<br>
> + }];<br>
> +}<br>
> +<br>
> +def LayoutVersionDocs : Documentation {<br>
> + let Category = DocCatType;<br>
> + let Content = [{<br>
> +The layout_version attribute requests that the compiler utilize the class<br>
> +layout rules of a particular compiler version.<br>
> +It is only supported when using the Microsoft C++ ABI.<br>
> + }];<br>
<br>
</div></div>Can you link to MSDN documentation for these two attributes to provide<br>
a bit more information? If not, it would be good to point out that<br>
both of these only apply to struct, class, and union types, and any<br>
other constraints that may be of interest to the user.<br></blockquote><div><br></div><div>Microsoft hasn't updated the list of documented declspecs for quite some time. I'll amend our documentation.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">
<div><div><br>
> +}<br>
> +<br>
> def MSInheritanceDocs : Documentation {<br>
> let Category = DocCatType;<br>
> let Heading = "__single_inhertiance, __multiple_inheritance, __virtual_inheritance";<br>
><br>
> Modified: cfe/trunk/lib/AST/RecordLayout.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/RecordLayout.cpp?rev=270457&r1=270456&r2=270457&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/RecordLayout.cpp?rev=270457&r1=270456&r2=270457&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/lib/AST/RecordLayout.cpp (original)<br>
> +++ cfe/trunk/lib/AST/RecordLayout.cpp Mon May 23 12:16:12 2016<br>
> @@ -58,7 +58,7 @@ ASTRecordLayout::ASTRecordLayout(const A<br>
> const CXXRecordDecl *PrimaryBase,<br>
> bool IsPrimaryBaseVirtual,<br>
> const CXXRecordDecl *BaseSharingVBPtr,<br>
> - bool HasZeroSizedSubObject,<br>
> + bool EndsWithZeroSizedObject,<br>
> bool LeadsWithZeroSizedBase,<br>
> const BaseOffsetsMapTy& BaseOffsets,<br>
> const VBaseOffsetsMapTy& VBaseOffsets)<br>
> @@ -82,7 +82,7 @@ ASTRecordLayout::ASTRecordLayout(const A<br>
> CXXInfo->VBPtrOffset = vbptroffset;<br>
> CXXInfo->HasExtendableVFPtr = hasExtendableVFPtr;<br>
> CXXInfo->BaseSharingVBPtr = BaseSharingVBPtr;<br>
> - CXXInfo->HasZeroSizedSubObject = HasZeroSizedSubObject;<br>
> + CXXInfo->EndsWithZeroSizedObject = EndsWithZeroSizedObject;<br>
> CXXInfo->LeadsWithZeroSizedBase = LeadsWithZeroSizedBase;<br>
><br>
><br>
><br>
> Modified: cfe/trunk/lib/AST/RecordLayoutBuilder.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/RecordLayoutBuilder.cpp?rev=270457&r1=270456&r2=270457&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/RecordLayoutBuilder.cpp?rev=270457&r1=270456&r2=270457&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/lib/AST/RecordLayoutBuilder.cpp (original)<br>
> +++ cfe/trunk/lib/AST/RecordLayoutBuilder.cpp Mon May 23 12:16:12 2016<br>
> @@ -2228,7 +2228,8 @@ public:<br>
> /// laid out.<br>
> void initializeCXXLayout(const CXXRecordDecl *RD);<br>
> void layoutNonVirtualBases(const CXXRecordDecl *RD);<br>
> - void layoutNonVirtualBase(const CXXRecordDecl *BaseDecl,<br>
> + void layoutNonVirtualBase(const CXXRecordDecl *RD,<br>
> + const CXXRecordDecl *BaseDecl,<br>
> const ASTRecordLayout &BaseLayout,<br>
> const ASTRecordLayout *&PreviousBaseLayout);<br>
> void injectVFPtr(const CXXRecordDecl *RD);<br>
> @@ -2334,7 +2335,7 @@ MicrosoftRecordLayoutBuilder::getAdjuste<br>
> if (!MaxFieldAlignment.isZero())<br>
> Info.Alignment = std::min(Info.Alignment, MaxFieldAlignment);<br>
> // Track zero-sized subobjects here where it's already available.<br>
> - EndsWithZeroSizedObject = Layout.hasZeroSizedSubObject();<br>
> + EndsWithZeroSizedObject = Layout.endsWithZeroSizedObject();<br>
> // Respect required alignment, this is necessary because we may have adjusted<br>
> // the alignment in the case of pragam pack. Note that the required alignment<br>
> // doesn't actually apply to the struct alignment at this point.<br>
> @@ -2369,7 +2370,7 @@ MicrosoftRecordLayoutBuilder::getAdjuste<br>
> if (auto RT =<br>
> FD->getType()->getBaseElementTypeUnsafe()->getAs<RecordType>()) {<br>
> auto const &Layout = Context.getASTRecordLayout(RT->getDecl());<br>
> - EndsWithZeroSizedObject = Layout.hasZeroSizedSubObject();<br>
> + EndsWithZeroSizedObject = Layout.endsWithZeroSizedObject();<br>
> FieldRequiredAlignment = std::max(FieldRequiredAlignment,<br>
> Layout.getRequiredAlignment());<br>
> }<br>
> @@ -2502,7 +2503,7 @@ MicrosoftRecordLayoutBuilder::layoutNonV<br>
> LeadsWithZeroSizedBase = BaseLayout.leadsWithZeroSizedBase();<br>
> }<br>
> // Lay out the base.<br>
> - layoutNonVirtualBase(BaseDecl, BaseLayout, PreviousBaseLayout);<br>
> + layoutNonVirtualBase(RD, BaseDecl, BaseLayout, PreviousBaseLayout);<br>
> }<br>
> // Figure out if we need a fresh VFPtr for this class.<br>
> if (!PrimaryBase && RD->isDynamicClass())<br>
> @@ -2531,7 +2532,7 @@ MicrosoftRecordLayoutBuilder::layoutNonV<br>
> LeadsWithZeroSizedBase = BaseLayout.leadsWithZeroSizedBase();<br>
> }<br>
> // Lay out the base.<br>
> - layoutNonVirtualBase(BaseDecl, BaseLayout, PreviousBaseLayout);<br>
> + layoutNonVirtualBase(RD, BaseDecl, BaseLayout, PreviousBaseLayout);<br>
> VBPtrOffset = Bases[BaseDecl] + BaseLayout.getNonVirtualSize();<br>
> }<br>
> // Set our VBPtroffset if we know it at this point.<br>
> @@ -2543,15 +2544,32 @@ MicrosoftRecordLayoutBuilder::layoutNonV<br>
> }<br>
> }<br>
><br>
> +static bool recordUsesEBO(const RecordDecl *RD) {<br>
> + if (!isa<CXXRecordDecl>(RD))<br>
> + return false;<br>
> + if (RD->hasAttr<EmptyBasesAttr>())<br>
> + return true;<br>
> + if (auto *LVA = RD->getAttr<LayoutVersionAttr>())<br>
> + // TODO: Double check with the next version of MSVC.<br>
> + if (LVA->getVersion() <= LangOptions::MSVC2015)<br>
> + return false;<br>
> + // TODO: Some later version of MSVC will change the default behavior of the<br>
> + // compiler to enable EBO by default. When this happens, we will need an<br>
> + // additional isCompatibleWithMSVC check.<br>
> + return false;<br>
> +}<br>
> +<br>
> void MicrosoftRecordLayoutBuilder::layoutNonVirtualBase(<br>
> + const CXXRecordDecl *RD,<br>
> const CXXRecordDecl *BaseDecl,<br>
> const ASTRecordLayout &BaseLayout,<br>
> const ASTRecordLayout *&PreviousBaseLayout) {<br>
> // Insert padding between two bases if the left first one is zero sized or<br>
> // contains a zero sized subobject and the right is zero sized or one leads<br>
> // with a zero sized base.<br>
> - if (PreviousBaseLayout && PreviousBaseLayout->hasZeroSizedSubObject() &&<br>
> - BaseLayout.leadsWithZeroSizedBase())<br>
> + bool MDCUsesEBO = recordUsesEBO(RD);<br>
> + if (PreviousBaseLayout && PreviousBaseLayout->endsWithZeroSizedObject() &&<br>
> + BaseLayout.leadsWithZeroSizedBase() && !MDCUsesEBO)<br>
> Size++;<br>
> ElementInfo Info = getAdjustedElementInfo(BaseLayout);<br>
> CharUnits BaseOffset;<br>
> @@ -2560,14 +2578,23 @@ void MicrosoftRecordLayoutBuilder::layou<br>
> bool FoundBase = false;<br>
> if (UseExternalLayout) {<br>
> FoundBase = External.getExternalNVBaseOffset(BaseDecl, BaseOffset);<br>
> - if (FoundBase)<br>
> + if (FoundBase) {<br>
> assert(BaseOffset >= Size && "base offset already allocated");<br>
> + Size = BaseOffset;<br>
> + }<br>
> }<br>
><br>
> - if (!FoundBase)<br>
> - BaseOffset = Size.alignTo(Info.Alignment);<br>
> + if (!FoundBase) {<br>
> + if (MDCUsesEBO && BaseDecl->isEmpty() &&<br>
> + BaseLayout.getNonVirtualSize() == CharUnits::Zero()) {<br>
> + BaseOffset = CharUnits::Zero();<br>
> + } else {<br>
> + // Otherwise, lay the base out at the end of the MDC.<br>
> + BaseOffset = Size = Size.alignTo(Info.Alignment);<br>
> + }<br>
> + }<br>
> Bases.insert(std::make_pair(BaseDecl, BaseOffset));<br>
> - Size = BaseOffset + BaseLayout.getNonVirtualSize();<br>
> + Size += BaseLayout.getNonVirtualSize();<br>
> PreviousBaseLayout = &BaseLayout;<br>
> }<br>
><br>
> @@ -2746,8 +2773,9 @@ void MicrosoftRecordLayoutBuilder::layou<br>
> // with a zero sized base. The padding between virtual bases is 4<br>
> // bytes (in both 32 and 64 bits modes) and always involves rounding up to<br>
> // the required alignment, we don't know why.<br>
> - if ((PreviousBaseLayout && PreviousBaseLayout->hasZeroSizedSubObject() &&<br>
> - BaseLayout.leadsWithZeroSizedBase()) || HasVtordisp) {<br>
> + if ((PreviousBaseLayout && PreviousBaseLayout->endsWithZeroSizedObject() &&<br>
> + BaseLayout.leadsWithZeroSizedBase() && !recordUsesEBO(RD)) ||<br>
> + HasVtordisp) {<br>
> Size = Size.alignTo(VtorDispAlignment) + VtorDispSize;<br>
> Alignment = std::max(VtorDispAlignment, Alignment);<br>
> }<br>
> @@ -2785,8 +2813,10 @@ void MicrosoftRecordLayoutBuilder::final<br>
> Size = Size.alignTo(RoundingAlignment);<br>
> }<br>
> if (Size.isZero()) {<br>
> - EndsWithZeroSizedObject = true;<br>
> - LeadsWithZeroSizedBase = true;<br>
> + if (!recordUsesEBO(RD) || !cast<CXXRecordDecl>(RD)->isEmpty()) {<br>
> + EndsWithZeroSizedObject = true;<br>
> + LeadsWithZeroSizedBase = true;<br>
> + }<br>
> // Zero-sized structures have size equal to their alignment if a<br>
> // __declspec(align) came into play.<br>
> if (RequiredAlignment >= MinEmptyStructSize)<br>
> @@ -2919,7 +2949,7 @@ ASTContext::getASTRecordLayout(const Rec<br>
> NewEntry = new (*this) ASTRecordLayout(<br>
> *this, Builder.Size, Builder.Alignment, Builder.RequiredAlignment,<br>
> Builder.HasOwnVFPtr, Builder.HasOwnVFPtr || Builder.PrimaryBase,<br>
> - Builder.VBPtrOffset, Builder.NonVirtualSize,<br>
> + Builder.VBPtrOffset, Builder.DataSize,<br>
> Builder.FieldOffsets.data(), Builder.FieldOffsets.size(),<br>
> Builder.NonVirtualSize, Builder.Alignment, CharUnits::Zero(),<br>
> Builder.PrimaryBase, false, Builder.SharedVBPtrBase,<br>
><br>
> Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=270457&r1=270456&r2=270457&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=270457&r1=270456&r2=270457&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)<br>
> +++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Mon May 23 12:16:12 2016<br>
> @@ -4968,6 +4968,24 @@ static void handleX86ForceAlignArgPointe<br>
> Attr.getAttributeSpellingListIndex()));<br>
> }<br>
><br>
> +static void handleLayoutVersion(Sema &S, Decl *D, const AttributeList &Attr) {<br>
> + uint32_t Version;<br>
> + Expr *VersionExpr = static_cast<Expr *>(Attr.getArgAsExpr(0));<br>
> + if (!checkUInt32Argument(S, Attr, Attr.getArgAsExpr(0), Version))<br>
> + return;<br>
> +<br>
> + // TODO: Investigate what happens with the next major version of MSVC.<br>
> + if (Version != LangOptions::MSVC2015) {<br>
> + S.Diag(Attr.getLoc(), diag::err_attribute_argument_out_of_bounds)<br>
> + << Attr.getName() << Version << VersionExpr->getSourceRange();<br>
> + return;<br>
> + }<br>
<br>
</div></div>So this cannot accept minor version numbers? (Presumably because minor<br>
and patch versions should not be changing layouts, I suppose.)<br></blockquote><div><br></div><div>It cannot.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">
<div><div><br>
> +<br>
> + D->addAttr(::new (S.Context)<br>
> + LayoutVersionAttr(Attr.getRange(), S.Context, Version,<br>
> + Attr.getAttributeSpellingListIndex()));<br>
> +}<br>
> +<br>
> DLLImportAttr *Sema::mergeDLLImportAttr(Decl *D, SourceRange Range,<br>
> unsigned AttrSpellingListIndex) {<br>
> if (D->hasAttr<DLLExportAttr>()) {<br>
> @@ -5749,6 +5767,12 @@ static void ProcessDeclAttribute(Sema &S<br>
> break;<br>
><br>
> // Microsoft attributes:<br>
> + case AttributeList::AT_EmptyBases:<br>
> + handleSimpleAttribute<EmptyBasesAttr>(S, D, Attr);<br>
> + break;<br>
> + case AttributeList::AT_LayoutVersion:<br>
> + handleLayoutVersion(S, D, Attr);<br>
> + break;<br>
> case AttributeList::AT_MSNoVTable:<br>
> handleSimpleAttribute<MSNoVTableAttr>(S, D, Attr);<br>
> break;<br>
> @@ -5767,6 +5791,7 @@ static void ProcessDeclAttribute(Sema &S<br>
> case AttributeList::AT_Thread:<br>
> handleDeclspecThreadAttr(S, D, Attr);<br>
> break;<br>
> +<br>
<br>
</div></div>Spurious newline added.<br></blockquote><div><br></div><div>Spurious newline intended. the abi_tag attribute is most certainly _not_ a Microsoft attribute.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">
<div><div><br>
> case AttributeList::AT_AbiTag:<br>
> handleAbiTagAttr(S, D, Attr);<br>
> break;<br>
><br>
> Added: cfe/trunk/test/Layout/ms-x86-declspec-empty_bases.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Layout/ms-x86-declspec-empty_bases.cpp?rev=270457&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Layout/ms-x86-declspec-empty_bases.cpp?rev=270457&view=auto</a><br>
> ==============================================================================<br>
> --- cfe/trunk/test/Layout/ms-x86-declspec-empty_bases.cpp (added)<br>
> +++ cfe/trunk/test/Layout/ms-x86-declspec-empty_bases.cpp Mon May 23 12:16:12 2016<br>
> @@ -0,0 +1,266 @@<br>
> +// RUN: %clang_cc1 -fno-rtti -emit-llvm-only -triple i686-pc-win32 -fms-extensions -fdump-record-layouts -fsyntax-only %s 2>/dev/null \<br>
> +// RUN: | FileCheck %s<br>
> +// RUN: %clang_cc1 -fno-rtti -emit-llvm-only -triple x86_64-pc-win32 -fms-extensions -fdump-record-layouts -fsyntax-only %s 2>/dev/null \<br>
> +// RUN: | FileCheck %s<br>
> +<br>
> +namespace test1 {<br>
> +<br>
> +struct A {<br>
> + int a;<br>
> +};<br>
> +struct B {<br>
> + int b;<br>
> +};<br>
> +struct C {};<br>
> +struct __declspec(align(16)) D {};<br>
> +struct __declspec(empty_bases) X : A, D, B, C {<br>
> +};<br>
> +<br>
> +// CHECK: *** Dumping AST Record Layout<br>
> +// CHECK-NEXT: 0 | struct test1::A<br>
> +// CHECK-NEXT: 0 | int a<br>
> +// CHECK-NEXT: | [sizeof=4, align=4,<br>
> +// CHECK-NEXT: | nvsize=4, nvalign=4]<br>
> +<br>
> +// CHECK: *** Dumping AST Record Layout<br>
> +// CHECK-NEXT: 0 | struct test1::D (empty)<br>
> +// CHECK-NEXT: | [sizeof=16, align=16,<br>
> +// CHECK-NEXT: | nvsize=0, nvalign=16]<br>
> +<br>
> +// CHECK: *** Dumping AST Record Layout<br>
> +// CHECK-NEXT: 0 | struct test1::B<br>
> +// CHECK-NEXT: 0 | int b<br>
> +// CHECK-NEXT: | [sizeof=4, align=4,<br>
> +// CHECK-NEXT: | nvsize=4, nvalign=4]<br>
> +<br>
> +// CHECK: *** Dumping AST Record Layout<br>
> +// CHECK-NEXT: 0 | struct test1::C (empty)<br>
> +// CHECK-NEXT: | [sizeof=1, align=1,<br>
> +// CHECK-NEXT: | nvsize=0, nvalign=1]<br>
> +<br>
> +// CHECK: *** Dumping AST Record Layout<br>
> +// CHECK-NEXT: 0 | struct test1::X<br>
> +// CHECK-NEXT: 0 | struct test1::A (base)<br>
> +// CHECK-NEXT: 0 | int a<br>
> +// CHECK-NEXT: 0 | struct test1::D (base) (empty)<br>
> +// CHECK-NEXT: 0 | struct test1::C (base) (empty)<br>
> +// CHECK-NEXT: 4 | struct test1::B (base)<br>
> +// CHECK-NEXT: 4 | int b<br>
> +// CHECK-NEXT: | [sizeof=16, align=16,<br>
> +// CHECK-NEXT: | nvsize=16, nvalign=16]<br>
> +<br>
> +int _ = sizeof(X);<br>
> +}<br>
> +<br>
> +namespace test2 {<br>
> +struct A {<br>
> + int a;<br>
> +};<br>
> +struct __declspec(empty_bases) B {};<br>
> +struct C : A {<br>
> + B b;<br>
> +};<br>
> +<br>
> +struct D {};<br>
> +struct E {<br>
> + int e;<br>
> +};<br>
> +struct F : D, E {};<br>
> +<br>
> +struct G : C, F {};<br>
> +<br>
> +int _ = sizeof(G);<br>
> +<br>
> +// CHECK: *** Dumping AST Record Layout<br>
> +// CHECK-NEXT: 0 | struct test2::A<br>
> +// CHECK-NEXT: 0 | int a<br>
> +// CHECK-NEXT: | [sizeof=4, align=4,<br>
> +// CHECK-NEXT: | nvsize=4, nvalign=4]<br>
> +<br>
> +// CHECK: *** Dumping AST Record Layout<br>
> +// CHECK-NEXT: 0 | struct test2::B (empty)<br>
> +// CHECK-NEXT: | [sizeof=1, align=1,<br>
> +// CHECK-NEXT: | nvsize=0, nvalign=1]<br>
> +<br>
> +// CHECK: *** Dumping AST Record Layout<br>
> +// CHECK-NEXT: 0 | struct test2::C<br>
> +// CHECK-NEXT: 0 | struct test2::A (base)<br>
> +// CHECK-NEXT: 0 | int a<br>
> +// CHECK-NEXT: 4 | struct test2::B b (empty)<br>
> +// CHECK-NEXT: | [sizeof=8, align=4,<br>
> +// CHECK-NEXT: | nvsize=8, nvalign=4]<br>
> +<br>
> +// CHECK: *** Dumping AST Record Layout<br>
> +// CHECK-NEXT: 0 | struct test2::D (empty)<br>
> +// CHECK-NEXT: | [sizeof=1, align=1,<br>
> +// CHECK-NEXT: | nvsize=0, nvalign=1]<br>
> +<br>
> +// CHECK: *** Dumping AST Record Layout<br>
> +// CHECK-NEXT: 0 | struct test2::E<br>
> +// CHECK-NEXT: 0 | int e<br>
> +// CHECK-NEXT: | [sizeof=4, align=4,<br>
> +// CHECK-NEXT: | nvsize=4, nvalign=4]<br>
> +<br>
> +// CHECK: *** Dumping AST Record Layout<br>
> +// CHECK-NEXT: 0 | struct test2::F<br>
> +// CHECK-NEXT: 0 | struct test2::D (base) (empty)<br>
> +// CHECK-NEXT: 0 | struct test2::E (base)<br>
> +// CHECK-NEXT: 0 | int e<br>
> +// CHECK-NEXT: | [sizeof=4, align=4,<br>
> +// CHECK-NEXT: | nvsize=4, nvalign=4]<br>
> +<br>
> +// CHECK: *** Dumping AST Record Layout<br>
> +// CHECK-NEXT: 0 | struct test2::G<br>
> +// CHECK-NEXT: 0 | struct test2::C (base)<br>
> +// CHECK-NEXT: 0 | struct test2::A (base)<br>
> +// CHECK-NEXT: 0 | int a<br>
> +// CHECK-NEXT: 4 | struct test2::B b (empty)<br>
> +// CHECK-NEXT: 8 | struct test2::F (base)<br>
> +// CHECK-NEXT: 8 | struct test2::D (base) (empty)<br>
> +// CHECK-NEXT: 8 | struct test2::E (base)<br>
> +// CHECK-NEXT: 8 | int e<br>
> +// CHECK-NEXT: | [sizeof=12, align=4,<br>
> +// CHECK-NEXT: | nvsize=12, nvalign=4]<br>
> +}<br>
> +<br>
> +namespace test3 {<br>
> +struct A {<br>
> + int a;<br>
> +};<br>
> +struct B {};<br>
> +struct C : A {<br>
> + B b;<br>
> +};<br>
> +<br>
> +struct D {};<br>
> +struct E {<br>
> + int e;<br>
> +};<br>
> +struct F : D, E {};<br>
> +<br>
> +struct __declspec(empty_bases) G : C, F {};<br>
> +<br>
> +int _ = sizeof(G);<br>
> +<br>
> +// CHECK: *** Dumping AST Record Layout<br>
> +// CHECK-NEXT: 0 | struct test3::A<br>
> +// CHECK-NEXT: 0 | int a<br>
> +// CHECK-NEXT: | [sizeof=4, align=4,<br>
> +// CHECK-NEXT: | nvsize=4, nvalign=4]<br>
> +<br>
> +// CHECK: *** Dumping AST Record Layout<br>
> +// CHECK-NEXT: 0 | struct test3::B (empty)<br>
> +// CHECK-NEXT: | [sizeof=1, align=1,<br>
> +// CHECK-NEXT: | nvsize=0, nvalign=1]<br>
> +<br>
> +// CHECK: *** Dumping AST Record Layout<br>
> +// CHECK-NEXT: 0 | struct test3::C<br>
> +// CHECK-NEXT: 0 | struct test3::A (base)<br>
> +// CHECK-NEXT: 0 | int a<br>
> +// CHECK-NEXT: 4 | struct test3::B b (empty)<br>
> +// CHECK-NEXT: | [sizeof=8, align=4,<br>
> +// CHECK-NEXT: | nvsize=8, nvalign=4]<br>
> +<br>
> +// CHECK: *** Dumping AST Record Layout<br>
> +// CHECK-NEXT: 0 | struct test3::D (empty)<br>
> +// CHECK-NEXT: | [sizeof=1, align=1,<br>
> +// CHECK-NEXT: | nvsize=0, nvalign=1]<br>
> +<br>
> +// CHECK: *** Dumping AST Record Layout<br>
> +// CHECK-NEXT: 0 | struct test3::E<br>
> +// CHECK-NEXT: 0 | int e<br>
> +// CHECK-NEXT: | [sizeof=4, align=4,<br>
> +// CHECK-NEXT: | nvsize=4, nvalign=4]<br>
> +<br>
> +// CHECK: *** Dumping AST Record Layout<br>
> +// CHECK-NEXT: 0 | struct test3::F<br>
> +// CHECK-NEXT: 0 | struct test3::D (base) (empty)<br>
> +// CHECK-NEXT: 0 | struct test3::E (base)<br>
> +// CHECK-NEXT: 0 | int e<br>
> +// CHECK-NEXT: | [sizeof=4, align=4,<br>
> +// CHECK-NEXT: | nvsize=4, nvalign=4]<br>
> +<br>
> +// CHECK: *** Dumping AST Record Layout<br>
> +// CHECK-NEXT: 0 | struct test3::G<br>
> +// CHECK-NEXT: 0 | struct test3::C (base)<br>
> +// CHECK-NEXT: 0 | struct test3::A (base)<br>
> +// CHECK-NEXT: 0 | int a<br>
> +// CHECK-NEXT: 4 | struct test3::B b (empty)<br>
> +// CHECK-NEXT: 8 | struct test3::F (base)<br>
> +// CHECK-NEXT: 8 | struct test3::D (base) (empty)<br>
> +// CHECK-NEXT: 8 | struct test3::E (base)<br>
> +// CHECK-NEXT: 8 | int e<br>
> +// CHECK-NEXT: | [sizeof=12, align=4,<br>
> +// CHECK-NEXT: | nvsize=12, nvalign=4]<br>
> +}<br>
> +<br>
> +namespace test4 {<br>
> +struct A {<br>
> + int a;<br>
> +};<br>
> +struct B {};<br>
> +struct C : A {<br>
> + B b;<br>
> +};<br>
> +<br>
> +struct __declspec(empty_bases) D {};<br>
> +struct E {<br>
> + int e;<br>
> +};<br>
> +struct F : D, E {};<br>
> +<br>
> +struct G : C, F {};<br>
> +<br>
> +int _ = sizeof(G);<br>
> +<br>
> +// CHECK: *** Dumping AST Record Layout<br>
> +// CHECK-NEXT: 0 | struct test4::A<br>
> +// CHECK-NEXT: 0 | int a<br>
> +// CHECK-NEXT: | [sizeof=4, align=4,<br>
> +// CHECK-NEXT: | nvsize=4, nvalign=4]<br>
> +<br>
> +// CHECK: *** Dumping AST Record Layout<br>
> +// CHECK-NEXT: 0 | struct test4::B (empty)<br>
> +// CHECK-NEXT: | [sizeof=1, align=1,<br>
> +// CHECK-NEXT: | nvsize=0, nvalign=1]<br>
> +<br>
> +// CHECK: *** Dumping AST Record Layout<br>
> +// CHECK-NEXT: 0 | struct test4::C<br>
> +// CHECK-NEXT: 0 | struct test4::A (base)<br>
> +// CHECK-NEXT: 0 | int a<br>
> +// CHECK-NEXT: 4 | struct test4::B b (empty)<br>
> +// CHECK-NEXT: | [sizeof=8, align=4,<br>
> +// CHECK-NEXT: | nvsize=8, nvalign=4]<br>
> +<br>
> +// CHECK: *** Dumping AST Record Layout<br>
> +// CHECK-NEXT: 0 | struct test4::D (empty)<br>
> +// CHECK-NEXT: | [sizeof=1, align=1,<br>
> +// CHECK-NEXT: | nvsize=0, nvalign=1]<br>
> +<br>
> +// CHECK: *** Dumping AST Record Layout<br>
> +// CHECK-NEXT: 0 | struct test4::E<br>
> +// CHECK-NEXT: 0 | int e<br>
> +// CHECK-NEXT: | [sizeof=4, align=4,<br>
> +// CHECK-NEXT: | nvsize=4, nvalign=4]<br>
> +<br>
> +// CHECK: *** Dumping AST Record Layout<br>
> +// CHECK-NEXT: 0 | struct test4::F<br>
> +// CHECK-NEXT: 0 | struct test4::D (base) (empty)<br>
> +// CHECK-NEXT: 0 | struct test4::E (base)<br>
> +// CHECK-NEXT: 0 | int e<br>
> +// CHECK-NEXT: | [sizeof=4, align=4,<br>
> +// CHECK-NEXT: | nvsize=4, nvalign=4]<br>
> +<br>
> +// CHECK: *** Dumping AST Record Layout<br>
> +// CHECK-NEXT: 0 | struct test4::G<br>
> +// CHECK-NEXT: 0 | struct test4::C (base)<br>
> +// CHECK-NEXT: 0 | struct test4::A (base)<br>
> +// CHECK-NEXT: 0 | int a<br>
> +// CHECK-NEXT: 4 | struct test4::B b (empty)<br>
> +// CHECK-NEXT: 8 | struct test4::F (base)<br>
> +// CHECK-NEXT: 8 | struct test4::D (base) (empty)<br>
> +// CHECK-NEXT: 8 | struct test4::E (base)<br>
> +// CHECK-NEXT: 8 | int e<br>
> +// CHECK-NEXT: | [sizeof=12, align=4,<br>
> +// CHECK-NEXT: | nvsize=12, nvalign=4]<br>
> +}<br>
><br>
> Added: cfe/trunk/test/SemaCXX/ms-empty_bases.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/ms-empty_bases.cpp?rev=270457&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/ms-empty_bases.cpp?rev=270457&view=auto</a><br>
> ==============================================================================<br>
> --- cfe/trunk/test/SemaCXX/ms-empty_bases.cpp (added)<br>
> +++ cfe/trunk/test/SemaCXX/ms-empty_bases.cpp Mon May 23 12:16:12 2016<br>
> @@ -0,0 +1,7 @@<br>
> +// RUN: %clang_cc1 -triple i386-pc-win32 %s -fsyntax-only -verify -fms-extensions -Wno-microsoft -std=c++11<br>
> +<br>
> +struct __declspec(empty_bases) S {};<br>
> +enum __declspec(empty_bases) E {}; // expected-warning{{'empty_bases' attribute only applies to classes}}<br>
> +int __declspec(empty_bases) I; // expected-warning{{'empty_bases' attribute only applies to classes}}<br>
> +typedef struct T __declspec(empty_bases) U; // expected-warning{{'empty_bases' attribute only applies to classes}}<br>
> +auto z = []() __declspec(empty_bases) { return nullptr; }; // expected-warning{{'empty_bases' attribute only applies to classes}}<br>
<br>
</div></div>Missing a test that ensures the attribute accepts no arguments.<br></blockquote><div><br></div><div>Sure, I'll add one.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">
<span><br>
><br>
> Added: cfe/trunk/test/SemaCXX/ms-layout_version.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/ms-layout_version.cpp?rev=270457&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/ms-layout_version.cpp?rev=270457&view=auto</a><br>
> ==============================================================================<br>
> --- cfe/trunk/test/SemaCXX/ms-layout_version.cpp (added)<br>
> +++ cfe/trunk/test/SemaCXX/ms-layout_version.cpp Mon May 23 12:16:12 2016<br>
> @@ -0,0 +1,10 @@<br>
> +// RUN: %clang_cc1 -triple i386-pc-win32 %s -fsyntax-only -verify -fms-extensions -Wno-microsoft -std=c++11<br>
> +<br>
> +struct __declspec(layout_version(19)) S {};<br>
> +enum __declspec(layout_version(19)) E {}; // expected-warning{{'layout_version' attribute only applies to classes}}<br>
> +int __declspec(layout_version(19)) I; // expected-warning{{'layout_version' attribute only applies to classes}}<br>
> +typedef struct T __declspec(layout_version(19)) U; // expected-warning{{'layout_version' attribute only applies to classes}}<br>
> +auto z = []() __declspec(layout_version(19)) { return nullptr; }; // expected-warning{{'layout_version' attribute only applies to classes}}<br>
> +<br>
> +struct __declspec(layout_version(18)) X {}; // expected-error{{'layout_version' attribute parameter 18 is out of bounds}}<br>
> +struct __declspec(layout_version(20)) Y {}; // expected-error{{'layout_version' attribute parameter 20 is out of bounds}}<br>
<br>
</span>Missing a test that ensures the attribute still fails when given no arguments.<br></blockquote><div><br></div><div>Sure, I'll add one.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">
<span><font color="#888888"><br>
~Aaron<br>
</font></span></blockquote></div><br></div></div>