Merge pull request #27414 from brentdax/identify-yourself

[NFC] Explicitly control alignment of Identifiers

Author: beccadax

include/swift/AST/Identifier.h

@@ -56,8 +56,26 @@ class Identifier {
 
   const char *Pointer;
 
+public:
+  enum : size_t {
+    NumLowBitsAvailable = 2,
+    RequiredAlignment = 1 << NumLowBitsAvailable,
+    SpareBitMask = ((intptr_t)1 << NumLowBitsAvailable) - 1
+  };
+
+private:
   /// Constructor, only accessible by ASTContext, which handles the uniquing.
-  explicit Identifier(const char *Ptr) : Pointer(Ptr) {}
+  explicit Identifier(const char *Ptr) : Pointer(Ptr) {
+    assert(((uintptr_t)Ptr & SpareBitMask) == 0
+           && "Identifier pointer does not use any spare bits");
+  }
+
+  /// A type with the alignment expected of a valid \c Identifier::Pointer .
+  struct alignas(uint32_t) Aligner {};
+
+  static_assert(alignof(Aligner) >= RequiredAlignment,
+                "Identifier table will provide enough spare bits");
+
 public:
   explicit Identifier() : Pointer(nullptr) {}
 
@@ -153,12 +171,15 @@ class Identifier {
   bool operator<(Identifier RHS) const { return Pointer < RHS.Pointer; }
 
   static Identifier getEmptyKey() {
-    return Identifier((const char*)
-                      llvm::DenseMapInfo<const void*>::getEmptyKey());
+    uintptr_t Val = static_cast<uintptr_t>(-1);
+    Val <<= NumLowBitsAvailable;
+    return Identifier((const char*)Val);
   }
+
   static Identifier getTombstoneKey() {
-    return Identifier((const char*)
-                      llvm::DenseMapInfo<const void*>::getTombstoneKey());
+    uintptr_t Val = static_cast<uintptr_t>(-2);
+    Val <<= NumLowBitsAvailable;
+    return Identifier((const char*)Val);
   }
 
 private:
@@ -202,7 +223,7 @@ namespace llvm {
     static inline swift::Identifier getFromVoidPointer(void *P) {
       return swift::Identifier::getFromOpaquePointer(P);
     }
-    enum { NumLowBitsAvailable = 2 };
+    enum { NumLowBitsAvailable = swift::Identifier::NumLowBitsAvailable };
   };
 
 } // end namespace llvm
@@ -221,15 +242,15 @@ class DeclBaseName {
   };
 
 private:
-  /// In a special DeclName represenenting a subscript, this opaque pointer
+  /// In a special DeclName representing a subscript, this opaque pointer
   /// is used as the data of the base name identifier.
   /// This is an implementation detail that should never leak outside of
   /// DeclName.
-  static void *SubscriptIdentifierData;
+  static const Identifier::Aligner SubscriptIdentifierData;
   /// As above, for special constructor DeclNames.
-  static void *ConstructorIdentifierData;
+  static const Identifier::Aligner ConstructorIdentifierData;
   /// As above, for special destructor DeclNames.
-  static void *DestructorIdentifierData;
+  static const Identifier::Aligner DestructorIdentifierData;
 
   Identifier Ident;
 
@@ -239,23 +260,23 @@ class DeclBaseName {
   DeclBaseName(Identifier I) : Ident(I) {}
 
   static DeclBaseName createSubscript() {
-    return DeclBaseName(Identifier((const char *)SubscriptIdentifierData));
+    return DeclBaseName(Identifier((const char *)&SubscriptIdentifierData));
   }
 
   static DeclBaseName createConstructor() {
-    return DeclBaseName(Identifier((const char *)ConstructorIdentifierData));
+    return DeclBaseName(Identifier((const char *)&ConstructorIdentifierData));
   }
 
   static DeclBaseName createDestructor() {
-    return DeclBaseName(Identifier((const char *)DestructorIdentifierData));
+    return DeclBaseName(Identifier((const char *)&DestructorIdentifierData));
   }
 
   Kind getKind() const {
-    if (Ident.get() == SubscriptIdentifierData) {
+    if (Ident.get() == (const char *)&SubscriptIdentifierData) {
       return Kind::Subscript;
-    } else if (Ident.get() == ConstructorIdentifierData) {
+    } else if (Ident.get() == (const char *)&ConstructorIdentifierData) {
       return Kind::Constructor;
-    } else if (Ident.get() == DestructorIdentifierData) {
+    } else if (Ident.get() == (const char *)&DestructorIdentifierData) {
         return Kind::Destructor;
     } else {
       return Kind::Normal;
@@ -720,7 +741,7 @@ namespace llvm {
     static inline swift::DeclName getFromVoidPointer(void *ptr) {
       return swift::DeclName::getFromOpaqueValue(ptr);
     }
-    enum { NumLowBitsAvailable = 0 };
+    enum { NumLowBitsAvailable = PointerLikeTypeTraits<swift::DeclBaseName>::NumLowBitsAvailable - 2 };
   };
 
   // DeclNames hash just like pointers.

lib/AST/ASTContext.cpp

@@ -165,7 +165,8 @@ struct ASTContext::Implementation {
 
   // FIXME: This is a StringMap rather than a StringSet because StringSet
   // doesn't allow passing in a pre-existing allocator.
-  llvm::StringMap<char, llvm::BumpPtrAllocator&> IdentifierTable;
+  llvm::StringMap<Identifier::Aligner, llvm::BumpPtrAllocator&>
+  IdentifierTable;
 
   /// The declaration of Swift.AssignmentPrecedence.
   PrecedenceGroupDecl *AssignmentPrecedence = nullptr;
@@ -642,7 +643,8 @@ Identifier ASTContext::getIdentifier(StringRef Str) const {
   if (Str.data() == nullptr)
     return Identifier(nullptr);
 
-  auto I = getImpl().IdentifierTable.insert(std::make_pair(Str, char())).first;
+  auto pair = std::make_pair(Str, Identifier::Aligner());
+  auto I = getImpl().IdentifierTable.insert(pair).first;
   return Identifier(I->getKeyData());
 }
 

lib/AST/Identifier.cpp

@@ -21,12 +21,9 @@
 #include "clang/Basic/CharInfo.h"
 using namespace swift;
 
-void *DeclBaseName::SubscriptIdentifierData =
-    &DeclBaseName::SubscriptIdentifierData;
-void *DeclBaseName::ConstructorIdentifierData =
-    &DeclBaseName::ConstructorIdentifierData;
-void *DeclBaseName::DestructorIdentifierData =
-    &DeclBaseName::DestructorIdentifierData;
+constexpr const Identifier::Aligner DeclBaseName::SubscriptIdentifierData{};
+constexpr const Identifier::Aligner DeclBaseName::ConstructorIdentifierData{};
+constexpr const Identifier::Aligner DeclBaseName::DestructorIdentifierData{};
 
 raw_ostream &llvm::operator<<(raw_ostream &OS, Identifier I) {
   if (I.get() == nullptr)