[Clang][AST] Resolve FIXME: Remove ObjCObjectPointer from isSpecifierType

[gAlfonso-bit]

Summary

There is no reason to have this here, (since all tests pass) and it isn't even a specifier anyway. We can just treat it as a pointer instead.

[gAlfonso-bit]

Rebased to upstream

[dexonsmith]

I'm a bit hesitant to trust lack of tests as proof that old FIXMEs can be resolved safely. Can you add some background information about why this was originally a specifier, and why it's safe to fix it now? (If you don't know, I suggest looking through git-blame to figure it out (unless @ahatanak already knows?).)

clang/lib/AST/DeclPrinter.cpp

161–163

Seems unrelated to this commit?

clang/lib/AST/TypePrinter.cpp

313 ↗

(On Diff #373955)

Seems unrelated to this commit?

[gAlfonso-bit]

clang/lib/AST/DeclPrinter.cpp

161–163

Did it to quiet clang-tidy because I modified this function

[gAlfonso-bit]

This comment was removed by gAlfonso-bit.

[gAlfonso-bit]

In D110068#3013636, @dexonsmith wrote:

I'm a bit hesitant to trust lack of tests as proof that old FIXMEs can be resolved safely. Can you add some background information about why this was originally a specifier, and why it's safe to fix it now? (If you don't know, I suggest looking through git-blame to figure it out (unless @ahatanak already knows?).)

This was introduced in 2010, with multiple changes following. The ObjCObjectPointer type was at a time before ObjCObject was defined.

[ahatanak]

I'm not sure why ObjCObjectPointerType was originally a specifier, but this patch looks safe to me. ObjCObjectPointerType is never passed to TypePrinter::printBefore and QualType GetBaseType, which are the two functions that use isSpecifierType, AFAICT.

John, do you remember why it was necessary to make ObjCObjectPointerType a specifier type?

[rjmccall]

ObjCObjectPointerType used to have a much weirder representation for the builtin object types like id and Class, and it's possible that this was an artifact of that. Otherwise I'm not sure, sorry.

[ahatanak]

id is a typedef and @class is an ObjCInterface now, so it looks like it's safe to remove ObjCObjectPointer case then.

[gAlfonso-bit]

Any updates? I do not have commit permissions. @ahatanak

[gAlfonso-bit]

Rebased to main

[gAlfonso-bit]

This comment was removed by gAlfonso-bit.

clang/lib/AST/DeclPrinter.cpp

 }
 
 static QualType GetBaseType(QualType T) {
   // FIXME: This should be on the Type class!
   QualType BaseType = T;
   while (!BaseType->isSpecifierType()) {
     if (const PointerType *PTy = BaseType->getAs<PointerType>())
       BaseType = PTy->getPointeeType();
+    else if (const ObjCObjectPointerType *OPT =
+                 BaseType->getAs<ObjCObjectPointerType>())
+      BaseType = OPT->getPointeeType();
     else if (const BlockPointerType *BPy = BaseType->getAs<BlockPointerType>())
       BaseType = BPy->getPointeeType();
-    else if (const ArrayType* ATy = dyn_cast<ArrayType>(BaseType))
+    else if (const ArrayType *ATy = dyn_cast<ArrayType>(BaseType))
       BaseType = ATy->getElementType();
-    else if (const FunctionType* FTy = BaseType->getAs<FunctionType>())
+    else if (const FunctionType *FTy = BaseType->getAs<FunctionType>())
       BaseType = FTy->getReturnType();
     else if (const VectorType *VTy = BaseType->getAs<VectorType>())
       BaseType = VTy->getElementType();
     else if (const ReferenceType *RTy = BaseType->getAs<ReferenceType>())
       BaseType = RTy->getPointeeType();
     else if (const AutoType *ATy = BaseType->getAs<AutoType>())
       BaseType = ATy->getDeducedType();
     else if (const ParenType *PTy = BaseType->getAs<ParenType>())

clang/lib/AST/Type.cpp

bool Type::isSpecifierType() const {
   case TemplateTypeParm:
   case SubstTemplateTypeParm:
   case TemplateSpecialization:
   case Elaborated:
   case DependentName:
   case DependentTemplateSpecialization:
   case ObjCInterface:
   case ObjCObject:
-  case ObjCObjectPointer: // FIXME: object pointers aren't really specifiers
     return true;
   default:
     return false;
   }
 }
 
 ElaboratedTypeKeyword
 TypeWithKeyword::getKeywordForTypeSpec(unsigned TypeSpec) {