[PATCH] D50643: [AST] Pack the bits of TemplateSpecializationType into Type

2018-08-15 Thread Phabricator via Phabricator via cfe-commits
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

2018-08-15 Thread Bruno Ricci via Phabricator via cfe-commits
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

2018-08-13 Thread Bruno Ricci via Phabricator via cfe-commits
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

2018-08-13 Thread Erich Keane via Phabricator via cfe-commits
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

2018-08-13 Thread Bruno Ricci via Phabricator via cfe-commits
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

2018-08-13 Thread Erich Keane via Phabricator via cfe-commits
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

2018-08-13 Thread Bruno Ricci via Phabricator via cfe-commits
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