[SemaObjC] Warn on visibility attributes on an @implementation

[erik.pilkington]

Summary

D60542 added support for attributes on @implementations, 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

[ahatanak]

This seems fine to me unless there are cases where the user wants to use it on the implementation. I can't think of a case where that is needed.

[ahatanak]

What if someone wants to use the attribute on either the interface or the implementation but can't modify the header?

[aaron.ballman]

This seems reasonable to me. I agree that this feels like it's something that should be written on the declaration in the header, not just magically appear on the definition, but I'm not an expert in ObjC to know if there are use cases where this may be useful.

clang/lib/Sema/SemaDeclAttr.cpp

 TypeVisibilityAttr *
 Sema::mergeTypeVisibilityAttr(Decl *D, const AttributeCommonInfo &CI,
                               TypeVisibilityAttr::VisibilityType Vis) {
   return ::mergeVisibilityAttr<TypeVisibilityAttr>(*this, D, CI, Vis);
 }
 
 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;
   }
 
   // 'type_visibility' can only go on a type or namespace.
   if (isTypeVisibility &&
       !(isa<TagDecl>(D) ||
         isa<ObjCInterfaceDecl>(D) ||

clang/test/Parser/attributes.mm

 @end
 
 #define EXP __attribute__((visibility("default")))
 class EXP C {};
 EXP class C2 {}; // expected-warning {{attribute 'visibility' is ignored, place it after "class" to apply attribute to type declaration}}
 
 @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}}
 
 @protocol EXP P @end // expected-error {{postfix attributes are not allowed on Objective-C directives, place them in front of '@protocol'}}
 EXP @protocol P2 @end