[PATCH] D43392: [clang-tidy] Add Fuchsia checker for visibility attributes

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang-tidy/fuchsia/AddVisibilityCheck.cpp:68
+      diag(MatchedDecl->getLocStart(),
+           "visibility attribute not set for specified function")
+          << MatchedDecl
----------------
jakehehrlich wrote:
> aaron.ballman wrote:
> > How about: "function expected to be annotated with '%0' visibility"
> > 
> > I'm mostly worried about the case where the function has a visibility attribute, but it has the *wrong* visibility specified. The current wording would be confusing in that circumstance. Or is that not a scenario you care about, just that *any* visibility is specified on the function?
> The use case for this check is forcing code bases to carefully control what symbols are exported rather than just exporting everything. So if someone took the time to explicitly set the visibility of one of these symbols we care about then we should assume they knew what they were doing.
> 
> The specific use case I care about for this check is using -fvisibility=hidden and then checking to make sure a certain curated list of symbols has explicit default visibility.
Ah, so a mismatch is unimportant, that's good to know. How about: "function expected to be annotated with the 'visibility' attribute"?


https://reviews.llvm.org/D43392