!fixup address comments

Author: fhahn

clang/lib/CodeGen/CodeGenTBAA.cpp

@@ -205,16 +205,27 @@ llvm::MDNode *CodeGenTBAA::getTypeInfoHelper(const Type *Ty) {
     llvm::MDNode *AnyPtr = createScalarTypeNode("any pointer", getChar(), Size);
     if (!CodeGenOpts.PointerTBAA)
       return AnyPtr;
-    // Compute the depth of the pointer and generate a tag of the form "p<depth>
-    // <base type tag>". Look through pointer and array types to determine the
-    // base type.
+    // C++ [basic.lval]p11 permits objects to accessed through an l-value of
+    // similar type. Two types are similar under C++ [conv.qual]p2 if the
+    // decomposition of the types into pointers, member pointers, and arrays has
+    // the same structure when ignoring cv-qualifiers at each level of the
+    // decomposition. Meanwhile, C makes T(*)[] and T(*)[N] compatible, which
+    // would really complicate any attempt to distinguish pointers to arrays by
+    // their bounds. It's simpler, and much easier to explain to users, to
+    // simply treat all pointers to arrays as pointers to their element type for
+    // aliasing purposes. So when creating a TBAA tag for a pointer type, we
+    // recursively ignore both qualifiers and array types when decomposing the
+    // pointee type. The only meaningful remaining structure is the number of
+    // pointer types we encountered along the way, so we just produce the tag
+    // "p<depth> <base type tag>". If we do find a member pointer type, for now
+    // we just conservatively bail out with AnyPtr (below) rather than trying to
+    // create a tag that honors the similar-type rules while still
+    // distinguishing different kinds of member pointer.
     unsigned PtrDepth = 0;
     do {
       PtrDepth++;
-      Ty = Ty->isPointerType() ? Ty->getPointeeType().getTypePtr()
-                               : Ty->getArrayElementTypeNoTypeQual();
+      Ty = Ty->getPointeeType()->getBaseElementTypeUnsafe();
     } while (Ty->isPointerType() || Ty->isArrayType());
-    Ty = Context.getBaseElementType(QualType(Ty, 0)).getTypePtr();
     assert(!isa<VariableArrayType>(Ty));
     // When the underlying type is a builtin type, we compute the pointee type
     // string recursively, which is implicitly more forgiving than the standards
@@ -232,9 +243,10 @@ llvm::MDNode *CodeGenTBAA::getTypeInfoHelper(const Type *Ty) {
               ->getString();
       TyName = Name;
     } else {
-      // Be conservative if the type a MemberPointerType. Those would require
-      // stripping const-qualifiers inside the type.
-      if (Ty->isMemberPointerType())
+      // Be conservative if the type isn't a RecordType. We are  specifically
+      // required to do this for member pointers until we implement the
+      // similar-types rule.
+      if (!Ty->isRecordType())
         return AnyPtr;
 
       // For non-builtin types use the mangled name of the canonical type.