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

[gAlfonso-bit]

Summary

ObjCObjectPointer is NOT a type specifier, so remove it from the list

[gAlfonso-bit]

Rebased

[gAlfonso-bit]

Not sure about the timing of base patch, but maybe this patch is also candidate for llvm 13 (backport)?

[gAlfonso-bit]

Add ObjectPointerType to GetBaseType

[gAlfonso-bit]

Rebased

[gAlfonso-bit]

Rearranged if statements

[gAlfonso-bit]

Passed every test in check-all!

[gAlfonso-bit]

Rebased

[dgoldman]

Does this change the behavior of TypePrinter or DeclPrinter - not sure if there are existing tests for those with ObjC code?

[gAlfonso-bit]

In D107775#2964937, @dgoldman wrote:

Does this change the behavior of TypePrinter or DeclPrinter - not sure if there are existing tests for those with ObjC code?

I put a case for it in the declprinter so it is handled appropriately so it would get the pointee type, whereas before this was neglected.

Other than that, there should be no difference.

[dgoldman]

I put a case for it in the declprinter so it is handled appropriately so it would get the pointee type, whereas before this was neglected.

Other than that, there should be no difference.

It looks like DeclPrinter only uses that method here - so it shouldn't make a difference.

but TypePrinter does call isSpecifierType() so this changes that behavior, right? Can you add a test to TypePrinterTest for this change?

[gAlfonso-bit]

In D107775#2967134, @dgoldman wrote:

I put a case for it in the declprinter so it is handled appropriately so it would get the pointee type, whereas before this was neglected.

Other than that, there should be no difference.

It looks like DeclPrinter only uses that method here - so it shouldn't make a difference.

but TypePrinter does call isSpecifierType() so this changes that behavior, right? Can you add a test to TypePrinterTest for this change?

Like for the function itself or an integration test?

[gAlfonso-bit]

@dgoldman May I ask what tests?

[gAlfonso-bit]

Added test to TypePrinter

[gAlfonso-bit]

git-format

[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) {

clang/lib/AST/TypePrinter.cpp

 /// "int foo[10]" it prints "int ".
 void TypePrinter::printBefore(const Type *T,Qualifiers Quals, raw_ostream &OS) {
   if (Policy.SuppressSpecifiers && T->isSpecifierType())
     return;
 
   SaveAndRestore<bool> PrevPHIsEmpty(HasEmptyPlaceHolder);
 
   // Print qualifiers as appropriate.
-
   bool CanPrefixQualifiers = false;
   bool NeedARCStrongQualifier = false;
   CanPrefixQualifiers = canPrefixQualifiers(T, NeedARCStrongQualifier);
 
   if (CanPrefixQualifiers && !Quals.empty()) {
     if (NeedARCStrongQualifier) {
       IncludeStrongLifetimeRAII Strong(Policy);
       Quals.print(OS, Policy, /*appendSpaceIfNonEmpty=*/true);

clang/unittests/AST/TypePrinterTest.cpp

TEST(TypePrinter, TemplateId) {
 
   ASSERT_TRUE(PrintedTypeMatches(
       Code, {}, Matcher, "const Type<T> &",
       [](PrintingPolicy &Policy) { Policy.FullyQualifiedName = false; }));
 
   ASSERT_TRUE(PrintedTypeMatches(
       Code, {}, Matcher, "const N::Type<T> &",
       [](PrintingPolicy &Policy) { Policy.FullyQualifiedName = true; }));
-}
+}
+
+TEST(TypePrinter, SpecifierType) {
+
+  auto Qual = qualType().bind(ObjCObjectPointerType);
+
+  ASSERT_TRUE(Qual.getTypePtr().isSpecifierType() == false);
+}