arphaman added a comment. In D98450#2699970 <https://reviews.llvm.org/D98450#2699970>, @arphaman wrote:
> Sorry, getting back to this only now. > > In D98450#2621907 <https://reviews.llvm.org/D98450#2621907>, @aaron.ballman > wrote: > >> In D98450#2621877 <https://reviews.llvm.org/D98450#2621877>, @MForster wrote: >> >>> That's ok for me. I'd suggest to wait for Aaron's feedback, because IIRC it >>> was his suggestion in the first place to go to declref. >> >> I have the same concerns with this patch as I did with the initial one, see >> https://reviews.llvm.org/D84005#inline-775391 -- basically, we're doing a >> lookup here in SemaDeclAttr.cpp and saying "I found the identifier, >> everything's good", then storing the identifier into the attribute so we can >> look it up again later, at which point looking up the identifier may find >> something totally unrelated to what was found in SemaDeclAttr.cpp. >> >>> The attribute with declref is incompatible with Apple's SDKs, as the >>> declref at the point of use in the attribute might not be available due to >>> the availability annotations on the domain declaration. >> >> Can you help me to understand why this isn't the expected behavior? If you >> name something that's not available, it seems surprising to me that it would >> then be available but only as one particular attribute argument. > > 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, > }; > > 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. > > Do you think in this case it would make sense to try to teach clang to reason > about this by special casing the fact that the enum declaration should check > if the typedef is annotated with the correct availability? 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? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98450/new/ https://reviews.llvm.org/D98450 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
