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

2020-05-02 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. 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

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

2020-04-29 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added a comment. What if someone wants to use the attribute on either the interface or the implementation but can't modify the header? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79094/new/ https://reviews.llvm.org/D79094 ___ cf

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

2020-04-29 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added a comment. 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. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79094/new/ https://reviews.llvm.org/D79094 ___

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

2020-04-29 Thread 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 added support for attributes on `@implementation`s, but CodeGen always looks for visibility attributes