[PATCH] D98450: [clang] store an identifer instead of declref for ns_error_domain attribute

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D98450#2699970 <https://reviews.llvm.org/D98450#2699970>, @arphaman wrote:

> Your argument makes sense. The problem right now is that clang doesn't allow the name to be used even when the user marks up availability correctly trying to guard the use of the declaration. This stems from the combination of this macro in Foundation:
>
>   #define NS_ERROR_ENUM(_type, _name, _domain)  \
>     enum _name : _type _name; enum __attribute__((ns_error_domain(_domain))) _name : _type
>
> and the fact that when the user uses it like this:
>
>   API_AVAILABLE(ios(14.0)) API_UNAVAILABLE(macos, tvos)
>   typedef NS_ERROR_ENUM(NIErrorDomain, NIErrorCode) {
>       NIErrorA = 1,
>   };

Is there an argument missing to `NS_ERROR_ENUM`? I don't see it passing the domain.

> which is equivalent to:
>
>   __attribute__((availability(ios,introduced=14.0))) __attribute__((availability(macos,unavailable))) __attribute__((availability(tvos,unavailable)))
>   NSErrorDomain const NIErrorDomain;
>   
>   __attribute__((availability(ios,introduced=14.0))) __attribute__((availability(macos,unavailable))) __attribute__((availability(tvos,unavailable)))
>   typedef enum NIErrorCode : NSInteger NIErrorCode; enum __attribute__((ns_error_domain(NIErrorDomain))) NIErrorCode : NSInteger {
>        NIErrorA = 1
>   };
>
> In this case clang doesn't know about availability on the `NIErrorDomain` declaration as it's placed on the typedef, so it reports the diagnostic that `NIErrorDomain` is unavailable for macOS, even though from users perspective the use is guarded correctly by the `API_UNAVAILABLE(macos, tvOS)` before the NS_ERROR_ENUM macro.

Ah, thank you for the detailed explanation!

In D98450#2699990 <https://reviews.llvm.org/D98450#2699990>, @arphaman wrote:

> I'm thinking maybe we could have an attribute that transcribes the availability from the typedef to the enum, e.g.
>
>   #define NS_ERROR_ENUM(_type, _name, _domain)  \
>     enum _name : _type _name; enum __attribute__((transcribe_availability(_name))) __attribute__((ns_error_domain(_domain))) _name : _type
>
> in that case we could teach clang that the enum should take the availability attributes from the typedef and apply to the enum, which should solve the use problem in the `ns_error_domain` attribute. Or potentially clang could try to infer it without the new attribute just by detecting this code pattern. WDYT?

I am mostly ignorant of the availability attributes, but do you think this functionality would be generally useful for effectively being able to say "make this declaration as available as this other declaration"? e.g., if you have a structure and a function that are used together, you can say:

  struct __attribute__((availability(...))) S { ... };
  __attribute__((transcribe_availability(S))) void func(struct S  s);

If the attribute is generally useful, then I think it would be better to add an explicit attribute. That also helps tools like static analyzers or AST matchers to make the connection between the availabilities, which would be harder if the information was inferred by the frontend.


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

https://reviews.llvm.org/D98450