[PATCH] D79094: [SemaObjC] Warn on visibility attributes on an @implementation

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington created this revision.
erik.pilkington added reviewers: rjmccall, aaron.ballman.
Herald added subscribers: ributzka, dexonsmith, jkorous.

D60542 <https://reviews.llvm.org/D60542> added support for attributes on `@implementation`s, but CodeGen always looks for visibility attributes on the `@interface`, so these attributes just end up getting ignored. I think we could support them on implementations, but it seems like information that should really belong in the header. This patch explicitly ignores them with a warning.

rdar://60045616


https://reviews.llvm.org/D79094

Files:
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/Parser/attributes.mm


Index: clang/test/Parser/attributes.mm
===================================================================
--- clang/test/Parser/attributes.mm
+++ clang/test/Parser/attributes.mm
@@ -15,9 +15,11 @@
 
 @interface EXP I @end // expected-error {{postfix attributes are not allowed on Objective-C directives, place them in front of '@interface'}}
 EXP @interface I2 @end
+EXP @interface I2 (Cat) @end
 
 @implementation EXP I @end // expected-error-re {{postfix attributes are not allowed on Objective-C directives{{$}}}}
-EXP @implementation I2 @end
+EXP @implementation I2 @end // expected-warning{{attribute ignored}}
+EXP @implementation I2 (Cat) @end // expected-warning{{attribute ignored}}
 
 @class EXP OC; // expected-error-re {{postfix attributes are not allowed on Objective-C directives{{$}}}}
 EXP @class OC2; // expected-error {{prefix attribute must be followed by an interface, protocol, or implementation}}
Index: clang/lib/Sema/SemaDeclAttr.cpp
===================================================================
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -2570,8 +2570,9 @@
 
 static void handleVisibilityAttr(Sema &S, Decl *D, const ParsedAttr &AL,
                                  bool isTypeVisibility) {
-  // Visibility attributes don't mean anything on a typedef.
-  if (isa<TypedefNameDecl>(D)) {
+  // Visibility attributes don't mean anything on a typedef, and aren't
+  // permitted on an @implementation.
+  if (isa<TypedefNameDecl>(D) || isa<ObjCImplDecl>(D)) {
     S.Diag(AL.getRange().getBegin(), diag::warn_attribute_ignored) << AL;
     return;
   }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D79094.260922.patch
Type: text/x-patch
Size: 1615 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200429/ae3a8c8c/attachment.bin>