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

Fri Mar 12 05:10:04 PST 2021

aaron.ballman added a comment.

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.

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

https://reviews.llvm.org/D98450