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

[Alex Lorenz via Phabricator via cfe-commits]

arphaman created this revision.
arphaman added reviewers: steven_wu, MForster.
Herald added subscribers: ributzka, jkorous.
Herald added a reviewer: aaron.ballman.
arphaman requested review of this revision.

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. The users also can't fix this by applying
the availability attributes to the user of ns_error_domain as at that point the
availability diagnostic is already emitted. The use of identifier is ideal as
clang then doesn't need to do availability checking when its referenced in the attribute.


https://reviews.llvm.org/D98450

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/Sema/ns_error_enum.m


Index: clang/test/Sema/ns_error_enum.m
===================================================================
--- clang/test/Sema/ns_error_enum.m
+++ clang/test/Sema/ns_error_enum.m
@@ -53,7 +53,6 @@
 
 extern char *const WrongErrorDomainType;
 enum __attribute__((ns_error_domain(WrongErrorDomainType))) MyWrongErrorDomainType { MyWrongErrorDomain };
-// expected-error at -1{{domain argument 'WrongErrorDomainType' does not point to an NSString or CFString constant}}
 
 struct __attribute__((ns_error_domain(MyErrorDomain))) MyStructWithErrorDomain {};
 // expected-error at -1{{'ns_error_domain' attribute only applies to enums}}
@@ -68,7 +67,7 @@
 // expected-error at -1{{'ns_error_domain' attribute takes one argument}}
 
 typedef NS_ERROR_ENUM(unsigned char, MyErrorEnumInvalid, InvalidDomain) {
-	// expected-error at -1{{use of undeclared identifier 'InvalidDomain'}}
+	// expected-error at -1{{domain argument 'InvalidDomain' does not refer to global constant}}
 	MyErrFirstInvalid,
 	MyErrSecondInvalid,
 };
Index: clang/lib/Sema/SemaDeclAttr.cpp
===================================================================
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -5510,28 +5510,28 @@
 }
 
 static void handleNSErrorDomain(Sema &S, Decl *D, const ParsedAttr &AL) {
-  auto *E = AL.getArgAsExpr(0);
-  auto Loc = E ? E->getBeginLoc() : AL.getLoc();
+  IdentifierLoc *IdentLoc = AL.isArgIdent(0) ? AL.getArgAsIdent(0) : nullptr;
+  if (!IdentLoc || !IdentLoc->Ident) {
+    // Try to locate the argument directly
+    SourceLocation Loc = AL.getLoc();
+    if (AL.isArgExpr(0) && AL.getArgAsExpr(0))
+      Loc = AL.getArgAsExpr(0)->getBeginLoc();
 
-  auto *DRE = dyn_cast<DeclRefExpr>(AL.getArgAsExpr(0));
-  if (!DRE) {
     S.Diag(Loc, diag::err_nserrordomain_invalid_decl) << 0;
     return;
   }
 
-  auto *VD = dyn_cast<VarDecl>(DRE->getDecl());
-  if (!VD) {
-    S.Diag(Loc, diag::err_nserrordomain_invalid_decl) << 1 << DRE->getDecl();
+  // Verify that the identifier is a valid decl in the C decl namespace
+  LookupResult LR(S, DeclarationName(IdentLoc->Ident), SourceLocation(),
+                  Sema::LookupNameKind::LookupOrdinaryName);
+  if (!S.LookupName(LR, S.TUScope) || !LR.getAsSingle<VarDecl>()) {
+    S.Diag(IdentLoc->Loc, diag::err_nserrordomain_invalid_decl)
+        << 1 << IdentLoc->Ident;
     return;
   }
 
-  if (!isNSStringType(VD->getType(), S.Context) &&
-      !isCFStringType(VD->getType(), S.Context)) {
-    S.Diag(Loc, diag::err_nserrordomain_wrong_type) << VD;
-    return;
-  }
-
-  D->addAttr(::new (S.Context) NSErrorDomainAttr(S.Context, AL, VD));
+  D->addAttr(::new (S.Context)
+                 NSErrorDomainAttr(S.Context, AL, IdentLoc->Ident));
 }
 
 static void handleObjCBridgeAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -9683,8 +9683,6 @@
   " attributes">;
 def err_nserrordomain_invalid_decl : Error<
   "domain argument %select{|%1 }0does not refer to global constant">;
-def err_nserrordomain_wrong_type : Error<
-  "domain argument %0 does not point to an NSString or CFString constant">;
 
 def warn_nsconsumed_attribute_mismatch : Warning<
   err_nsconsumed_attribute_mismatch.Text>, InGroup<NSConsumedMismatch>;
Index: clang/include/clang/Basic/Attr.td
===================================================================
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -1967,7 +1967,7 @@
 def NSErrorDomain : InheritableAttr {
   let Spellings = [GNU<"ns_error_domain">];
   let Subjects = SubjectList<[Enum], ErrorDiag>;
-  let Args = [DeclArgument<Var, "ErrorDomain">];
+  let Args = [IdentifierArgument<"ErrorDomain">];
   let Documentation = [NSErrorDomainDocs];
 }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D98450.330062.patch
Type: text/x-patch
Size: 3923 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20210311/c7c219de/attachment.bin>