juliehockett added inline comments.
================ Comment at: clang-tidy/fuchsia/AddVisibilityCheck.cpp:68 + diag(MatchedDecl->getLocStart(), + "visibility attribute not set for specified function") + << MatchedDecl ---------------- aaron.ballman wrote: > 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"? So in implementing a way to handle #pragma and command line directives, it became clear that the more comprehensive check (that replaces existing attrs) was a trivial implementation on top of that, so I put it in. If we just want to ignore those cases, I can take out the conditional path. https://reviews.llvm.org/D43392 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
