__attribute__((format_arg(__NSString__, N))) does not support instancetype in NSString interface

[fcloutier]

Summary

It has come to my attention that _inside the interface of NSString_, using __attribute__((format_arg(__NSString__, N))) does not work for a method that returns instancetype. This is a pretty specific problem because not a lot of people define NSString. Thankfully, the fix isn't very difficult: if checking an Objective-C method definition, and it returns instancetype, we also check if the class being defined is NSString.

rdar://84729746

[fcloutier]

Forgot to run clang-format.

[ahatanak]

clang/lib/Sema/SemaDeclAttr.cpp

3390–3396

Is it possible to just replace Ty with the class pointer type here if it is an instancetype instead of defining another function (isNSStringInterface) that looks at the class name and determines whether it's NSString?

[fcloutier]

clang/lib/Sema/SemaDeclAttr.cpp

3390–3396

I think that it would be awkward and I'd like to offer modest pushback. You need the enclosing Decl to make sense of instancetype because it's just a typedef to id. There are three uses of isNSStringType and only one could benefit from the Decl it because the other two don't refer to a return type. IMO, the split is the right way to go.

FWIW, it's not so much defining another function that looks at the class name so much as moving the look-at-the-class-name logic out of an existing function. I took the guts out of isNSStringType to make isNSStringInterface, but Phabricator isn't very good at showing code that moved.

[fcloutier]

Apologies, Phabricator showed the comment below line 197 in the diff, but the email showed it to be below line 3404. I can check if the return type is instancetype in handleFormatArgAttr and use that instead.

[fcloutier]

Add test for a protocol method with format_arg, second NSString method accepting a NSString instead of a C string

[ahatanak]

LGTM

clang/lib/Sema/SemaDeclAttr.cpp

3390–3396

Oh I see. But I still feel this is better since isNSStringInterface would be called twice in the previous patch.

3392

You can use ASTContext::getObjCInstanceType, which returns QualType.

clang/lib/Sema/SemaDeclAttr.cpp

if (NotNSStringTy &&
       !isCFStringType(Ty, S.Context) &&
       (!Ty->isPointerType() ||
        !Ty->castAs<PointerType>()->getPointeeType()->isCharType())) {
     S.Diag(AL.getLoc(), diag::err_format_attribute_not)
         << "a string type" << IdxExpr->getSourceRange()
         << getFunctionOrMethodParamRange(D, 0);
     return;
   }
   Ty = getFunctionOrMethodResultType(D);
+  // replace instancetype with the class type
+  if (Ty.getTypePtr() == S.Context.getObjCInstanceTypeDecl()->getTypeForDecl())
+    if (auto *OMD = dyn_cast<ObjCMethodDecl>(D))
+      if (auto *Interface = OMD->getClassInterface())
+        Ty = S.Context.getObjCObjectPointerType(
+            QualType(Interface->getTypeForDecl(), 0));
   if (!isNSStringType(Ty, S.Context, /*AllowNSAttributedString=*/true) &&
       !isCFStringType(Ty, S.Context) &&
       (!Ty->isPointerType() ||
        !Ty->castAs<PointerType>()->getPointeeType()->isCharType())) {
     S.Diag(AL.getLoc(), diag::err_format_attribute_result_not)
         << (NotNSStringTy ? "string type" : "NSString")
         << IdxExpr->getSourceRange() << getFunctionOrMethodParamRange(D, 0);
     return;

clang/test/SemaObjC/format-arg-attribute.m

 // RUN: %clang_cc1 -verify -fsyntax-only %s
 
-@class NSString;
+@interface NSString
++(instancetype)stringWithCString:(const char *)cstr __attribute__((format_arg(1)));
++(instancetype)stringWithString:(NSString *)cstr __attribute__((format_arg(1)));
+@end
+
+@protocol MaybeString
+-(instancetype)maybeString:(const char *)cstr __attribute__((format_arg(1))); // expected-error {{function does not return string type}}
+@end
+
 @class NSAttributedString;
 
 extern NSString *fa2 (const NSString *) __attribute__((format_arg(1)));
 extern NSString *fa3 (NSString *) __attribute__((format_arg(1)));
 
 extern void fc1 (const NSString *) __attribute__((format_arg));  // expected-error {{'format_arg' attribute takes one argument}}
 extern void fc2 (const NSString *) __attribute__((format_arg())); // expected-error {{'format_arg' attribute takes one argument}}
 extern void fc3 (const NSString *) __attribute__((format_arg(1, 2))); // expected-error {{'format_arg' attribute takes one argument}}