[PATCH] D112670: __attribute__((format_arg(__NSString__, N))) does not support instancetype in NSString interface

[Félix Cloutier via Phabricator via cfe-commits]

fcloutier added inline comments.


================
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:3403
   }
   Ty = getFunctionOrMethodResultType(D);
   if (!isNSStringType(Ty, S.Context, /*AllowNSAttributedString=*/true) &&
----------------
ahatanak wrote:
> 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`?
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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112670/new/

https://reviews.llvm.org/D112670