[PATCH] D50643: [AST] Pack the bits of TemplateSpecializationType into Type
This revision was automatically updated to reflect the committed changes. Closed by commit rL339787: [AST] Pack the bits of TemplateSpecializationType into Type (authored by brunoricci, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D50643?vs=160805=160828#toc Repository: rL LLVM https://reviews.llvm.org/D50643 Files: cfe/trunk/include/clang/AST/Type.h cfe/trunk/lib/AST/Type.cpp Index: cfe/trunk/lib/AST/Type.cpp === --- cfe/trunk/lib/AST/Type.cpp +++ cfe/trunk/lib/AST/Type.cpp @@ -3335,8 +3335,10 @@ Canon.isNull()? true : Canon->isDependentType(), Canon.isNull()? true : Canon->isInstantiationDependentType(), false, - T.containsUnexpandedParameterPack()), -Template(T), NumArgs(Args.size()), TypeAlias(!AliasedType.isNull()) { + T.containsUnexpandedParameterPack()), Template(T) { + TemplateSpecializationTypeBits.NumArgs = Args.size(); + TemplateSpecializationTypeBits.TypeAlias = !AliasedType.isNull(); + assert(!T.getAsDependentTemplateName() && "Use DependentTemplateSpecializationType for dependent template-name"); assert((T.getKind() == TemplateName::Template || @@ -3365,7 +3367,7 @@ } // Store the aliased type if this is a type alias template specialization. - if (TypeAlias) { + if (isTypeAlias()) { auto *Begin = reinterpret_cast(this + 1); *reinterpret_cast(Begin + getNumArgs()) = AliasedType; } Index: cfe/trunk/include/clang/AST/Type.h === --- cfe/trunk/include/clang/AST/Type.h +++ cfe/trunk/include/clang/AST/Type.h @@ -1606,6 +1606,24 @@ unsigned Keyword : 2; }; + class TemplateSpecializationTypeBitfields { +friend class TemplateSpecializationType; + +unsigned : NumTypeBits; + +/// Whether this template specialization type is a substituted type alias. +unsigned TypeAlias : 1; + +/// The number of template arguments named in this class template +/// specialization, which is expected to be able to hold at least 1024 +/// according to [implimits]. However, as this limit is somewhat easy to +/// hit with template metaprogramming we'd prefer to keep it as large +/// as possible. At the moment it has been left as a non-bitfield since +/// this type safely fits in 64 bits as an unsigned, so there is no reason +/// to introduce the performance impact of a bitfield. +unsigned NumArgs; + }; + union { TypeBitfields TypeBits; ArrayTypeBitfields ArrayTypeBits; @@ -1617,6 +1635,7 @@ ReferenceTypeBitfields ReferenceTypeBits; TypeWithKeywordBitfields TypeWithKeywordBits; VectorTypeBitfields VectorTypeBits; +TemplateSpecializationTypeBitfields TemplateSpecializationTypeBits; static_assert(sizeof(TypeBitfields) <= 8, "TypeBitfields is larger than 8 bytes!"); @@ -1638,6 +1657,9 @@ "TypeWithKeywordBitfields is larger than 8 bytes!"); static_assert(sizeof(VectorTypeBitfields) <= 8, "VectorTypeBitfields is larger than 8 bytes!"); +static_assert(sizeof(TemplateSpecializationTypeBitfields) <= 8, + "TemplateSpecializationTypeBitfields is larger" + " than 8 bytes!"); }; private: @@ -4669,13 +4691,6 @@ /// replacement must, recursively, be one of these). TemplateName Template; - /// The number of template arguments named in this class template - /// specialization. - unsigned NumArgs : 31; - - /// Whether this template specialization type is a substituted type alias. - unsigned TypeAlias : 1; - TemplateSpecializationType(TemplateName T, ArrayRef Args, QualType Canon, @@ -4710,7 +4725,7 @@ /// typedef A type; // not a type alias /// }; /// \endcode - bool isTypeAlias() const { return TypeAlias; } + bool isTypeAlias() const { return TemplateSpecializationTypeBits.TypeAlias; } /// Get the aliased type, if this is a specialization of a type alias /// template. @@ -4733,14 +4748,16 @@ } /// Retrieve the number of template arguments. - unsigned getNumArgs() const { return NumArgs; } + unsigned getNumArgs() const { +return TemplateSpecializationTypeBits.NumArgs; + } /// Retrieve a specific template argument as a type. /// \pre \c isArgType(Arg) const TemplateArgument (unsigned Idx) const; // in TemplateBase.h ArrayRef template_arguments() const { -return {getArgs(), NumArgs}; +return {getArgs(), getNumArgs()}; } bool isSugared() const { ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50643: [AST] Pack the bits of TemplateSpecializationType into Type
riccibruno updated this revision to Diff 160805. riccibruno added a comment. update the comment in TemplateSpecializationTypeBitfields Repository: rC Clang https://reviews.llvm.org/D50643 Files: include/clang/AST/Type.h lib/AST/Type.cpp Index: lib/AST/Type.cpp === --- lib/AST/Type.cpp +++ lib/AST/Type.cpp @@ -3335,8 +3335,10 @@ Canon.isNull()? true : Canon->isDependentType(), Canon.isNull()? true : Canon->isInstantiationDependentType(), false, - T.containsUnexpandedParameterPack()), -Template(T), NumArgs(Args.size()), TypeAlias(!AliasedType.isNull()) { + T.containsUnexpandedParameterPack()), Template(T) { + TemplateSpecializationTypeBits.NumArgs = Args.size(); + TemplateSpecializationTypeBits.TypeAlias = !AliasedType.isNull(); + assert(!T.getAsDependentTemplateName() && "Use DependentTemplateSpecializationType for dependent template-name"); assert((T.getKind() == TemplateName::Template || @@ -3365,7 +3367,7 @@ } // Store the aliased type if this is a type alias template specialization. - if (TypeAlias) { + if (isTypeAlias()) { auto *Begin = reinterpret_cast(this + 1); *reinterpret_cast(Begin + getNumArgs()) = AliasedType; } Index: include/clang/AST/Type.h === --- include/clang/AST/Type.h +++ include/clang/AST/Type.h @@ -1624,6 +1624,24 @@ unsigned Keyword : 2; }; + class TemplateSpecializationTypeBitfields { +friend class TemplateSpecializationType; + +unsigned : NumTypeBits; + +/// Whether this template specialization type is a substituted type alias. +unsigned TypeAlias : 1; + +/// The number of template arguments named in this class template +/// specialization, which is expected to be able to hold at least 1024 +/// according to [implimits]. However, as this limit is somewhat easy to +/// hit with template metaprogramming we'd prefer to keep it as large +/// as possible. At the moment it has been left as a non-bitfield since +/// this type safely fits in 64 bits as an unsigned, so there is no reason +/// to introduce the performance impact of a bitfield. +unsigned NumArgs; + }; + union { TypeBitfields TypeBits; ArrayTypeBitfields ArrayTypeBits; @@ -1635,6 +1653,7 @@ ReferenceTypeBitfields ReferenceTypeBits; TypeWithKeywordBitfields TypeWithKeywordBits; VectorTypeBitfields VectorTypeBits; +TemplateSpecializationTypeBitfields TemplateSpecializationTypeBits; static_assert(sizeof(TypeBitfields) <= 8, "TypeBitfields is larger than 8 bytes!"); @@ -1656,6 +1675,9 @@ "TypeWithKeywordBitfields is larger than 8 bytes!"); static_assert(sizeof(VectorTypeBitfields) <= 8, "VectorTypeBitfields is larger than 8 bytes!"); +static_assert(sizeof(TemplateSpecializationTypeBitfields) <= 8, + "TemplateSpecializationTypeBitfields is larger" + " than 8 bytes!"); }; private: @@ -4673,13 +4695,6 @@ /// replacement must, recursively, be one of these). TemplateName Template; - /// The number of template arguments named in this class template - /// specialization. - unsigned NumArgs : 31; - - /// Whether this template specialization type is a substituted type alias. - unsigned TypeAlias : 1; - TemplateSpecializationType(TemplateName T, ArrayRef Args, QualType Canon, @@ -4714,7 +4729,7 @@ /// typedef A type; // not a type alias /// }; /// \endcode - bool isTypeAlias() const { return TypeAlias; } + bool isTypeAlias() const { return TemplateSpecializationTypeBits.TypeAlias; } /// Get the aliased type, if this is a specialization of a type alias /// template. @@ -4737,14 +4752,16 @@ } /// Retrieve the number of template arguments. - unsigned getNumArgs() const { return NumArgs; } + unsigned getNumArgs() const { +return TemplateSpecializationTypeBits.NumArgs; + } /// Retrieve a specific template argument as a type. /// \pre \c isArgType(Arg) const TemplateArgument (unsigned Idx) const; // in TemplateBase.h ArrayRef template_arguments() const { -return {getArgs(), NumArgs}; +return {getArgs(), getNumArgs()}; } bool isSugared() const { ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50643: [AST] Pack the bits of TemplateSpecializationType into Type
riccibruno added a comment. I don't like these bit-field abuses too much but this is used all over the place in clang and so if this breaks so many things are going to break. Repository: rC Clang https://reviews.llvm.org/D50643 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50643: [AST] Pack the bits of TemplateSpecializationType into Type
erichkeane accepted this revision. erichkeane added a comment. This revision is now accepted and ready to land. Ah, right. I missed that the others already do it. Fine I guess... Repository: rC Clang https://reviews.llvm.org/D50643 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50643: [AST] Pack the bits of TemplateSpecializationType into Type
riccibruno added a comment. All of these bitfields (ab)use are already UB I think... I don't see what is special in this case... Repository: rC Clang https://reviews.llvm.org/D50643 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50643: [AST] Pack the bits of TemplateSpecializationType into Type
erichkeane added a comment. I think that this causes UB. The Type baseclass will use the TypeBitfields active member, but this uses the TemplateSpecializationTypeBits, right? I know we take advantage of this elsewhere, but I'm tentative about this one... Repository: rC Clang https://reviews.llvm.org/D50643 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50643: [AST] Pack the bits of TemplateSpecializationType into Type
riccibruno created this revision. riccibruno added a reviewer: erichkeane. riccibruno added a project: clang. Herald added a subscriber: cfe-commits. Type has enough space for two members of TemplateSpecializationType. Mechanical patch. Repository: rC Clang https://reviews.llvm.org/D50643 Files: include/clang/AST/Type.h lib/AST/Type.cpp Index: lib/AST/Type.cpp === --- lib/AST/Type.cpp +++ lib/AST/Type.cpp @@ -3335,8 +3335,10 @@ Canon.isNull()? true : Canon->isDependentType(), Canon.isNull()? true : Canon->isInstantiationDependentType(), false, - T.containsUnexpandedParameterPack()), -Template(T), NumArgs(Args.size()), TypeAlias(!AliasedType.isNull()) { + T.containsUnexpandedParameterPack()), Template(T) { + TemplateSpecializationTypeBits.NumArgs = Args.size(); + TemplateSpecializationTypeBits.TypeAlias = !AliasedType.isNull(); + assert(!T.getAsDependentTemplateName() && "Use DependentTemplateSpecializationType for dependent template-name"); assert((T.getKind() == TemplateName::Template || @@ -3365,7 +3367,7 @@ } // Store the aliased type if this is a type alias template specialization. - if (TypeAlias) { + if (isTypeAlias()) { auto *Begin = reinterpret_cast(this + 1); *reinterpret_cast(Begin + getNumArgs()) = AliasedType; } Index: include/clang/AST/Type.h === --- include/clang/AST/Type.h +++ include/clang/AST/Type.h @@ -1624,6 +1624,20 @@ unsigned Keyword : 2; }; + class TemplateSpecializationTypeBitfields { +friend class TemplateSpecializationType; + +unsigned : NumTypeBits; + +/// Whether this template specialization type is a substituted type alias. +unsigned TypeAlias : 1; + +/// The number of template arguments named in this class template +/// specialization. Intentionally not a bitfield since we have +/// plenty of space left. +unsigned NumArgs; + }; + union { TypeBitfields TypeBits; ArrayTypeBitfields ArrayTypeBits; @@ -1635,6 +1649,7 @@ ReferenceTypeBitfields ReferenceTypeBits; TypeWithKeywordBitfields TypeWithKeywordBits; VectorTypeBitfields VectorTypeBits; +TemplateSpecializationTypeBitfields TemplateSpecializationTypeBits; static_assert(sizeof(TypeBitfields) <= 8, "TypeBitfields is larger than 8 bytes!"); @@ -1656,6 +1671,9 @@ "TypeWithKeywordBitfields is larger than 8 bytes!"); static_assert(sizeof(VectorTypeBitfields) <= 8, "VectorTypeBitfields is larger than 8 bytes!"); +static_assert(sizeof(TemplateSpecializationTypeBitfields) <= 8, + "TemplateSpecializationTypeBitfields is larger" + " than 8 bytes!"); }; private: @@ -4673,13 +4691,6 @@ /// replacement must, recursively, be one of these). TemplateName Template; - /// The number of template arguments named in this class template - /// specialization. - unsigned NumArgs : 31; - - /// Whether this template specialization type is a substituted type alias. - unsigned TypeAlias : 1; - TemplateSpecializationType(TemplateName T, ArrayRef Args, QualType Canon, @@ -4714,7 +4725,7 @@ /// typedef A type; // not a type alias /// }; /// \endcode - bool isTypeAlias() const { return TypeAlias; } + bool isTypeAlias() const { return TemplateSpecializationTypeBits.TypeAlias; } /// Get the aliased type, if this is a specialization of a type alias /// template. @@ -4737,14 +4748,16 @@ } /// Retrieve the number of template arguments. - unsigned getNumArgs() const { return NumArgs; } + unsigned getNumArgs() const { +return TemplateSpecializationTypeBits.NumArgs; + } /// Retrieve a specific template argument as a type. /// \pre \c isArgType(Arg) const TemplateArgument (unsigned Idx) const; // in TemplateBase.h ArrayRef template_arguments() const { -return {getArgs(), NumArgs}; +return {getArgs(), getNumArgs()}; } bool isSugared() const { ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits