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

2018-02-27 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang-tidy/fuchsia/AddVisibilityCheck.cpp:52-53 + Finder->addMatcher( + functionDecl(allOf(hasAnyName(SmallVector(Names.begin(), + Names.end())), +

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

2018-02-26 Thread Jake Ehrlich via Phabricator via cfe-commits
jakehehrlich added a comment. Can you add a test for when #pragma GCC visibility push(hidden) is used and the visibility attribute is hidden? https://reviews.llvm.org/D43392 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.

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

2018-02-26 Thread Jake Ehrlich via Phabricator via cfe-commits
jakehehrlich added inline comments. Comment at: clang-tidy/fuchsia/AddVisibilityCheck.cpp:26 + +// AST_MATCHER(FunctionDecl, isInHeaderFile) { +// return Node.getExplicitVisibility(NamedDecl::VisibilityForType); What are these comments doing here? ===

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

2018-02-26 Thread Julie Hockett via Phabricator via cfe-commits
juliehockett updated this revision to Diff 135944. juliehockett marked 6 inline comments as done. juliehockett added a comment. After discussion, the goal of this checker slightly changed to target definitions in header files, rather than declarations. As a result, the check now adds the attribu

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

2018-02-26 Thread Julie Hockett via Phabricator via cfe-commits
juliehockett added inline comments. Comment at: clang-tidy/fuchsia/AddVisibilityCheck.cpp:52-53 + Finder->addMatcher( + functionDecl(allOf(hasAnyName(SmallVector(Names.begin(), + Names.end())), +

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

2018-02-21 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang-tidy/fuchsia/AddVisibilityCheck.cpp:23-25 + // if (const auto *D = Node.getDefinition()) { + // return D->getExplicitVisibility(NamedDecl::VisibilityForType) == V; + // } Can remove these comments. ===

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

2018-02-20 Thread Julie Hockett via Phabricator via cfe-commits
juliehockett updated this revision to Diff 135181. juliehockett marked 10 inline comments as done. juliehockett added a comment. Updating check to allow for #pragma and command line visibility specs, and updating tests https://reviews.llvm.org/D43392 Files: clang-tidy/fuchsia/AddVisibilityCh

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

2018-02-20 Thread Julie Hockett via Phabricator via cfe-commits
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 w

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

2018-02-17 Thread 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

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

2018-02-16 Thread Jake Ehrlich via Phabricator via cfe-commits
jakehehrlich 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: > How about: "fu

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

2018-02-16 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang-tidy/fuchsia/AddVisibilityCheck.cpp:36-39 + StringRef(Options.get("Names", "")).split(NamesRefs, ","); + for (const auto &Name : NamesRefs) { +if (!Name.empty()) Names.push_back(Name); + } Rather than

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

2018-02-16 Thread Julie Hockett via Phabricator via cfe-commits
juliehockett updated this revision to Diff 134684. juliehockett marked 7 inline comments as done. juliehockett edited the summary of this revision. juliehockett added a comment. 1. Added visibility parameter 2. Updated Name parameter to be a list of names, as opposed to a single name 3. Fixed comm

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

2018-02-16 Thread Jake Ehrlich via Phabricator via cfe-commits
jakehehrlich added a comment. > I'd like a test for #pragma GCC visibility push(hidden) which should cause > each symbol to be treated as though it were explicitly hidden. To clarify, this check should still give a warning if the specified variable doesn't actually have that in source code. e.g

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

2018-02-16 Thread Jake Ehrlich via Phabricator via cfe-commits
jakehehrlich added a comment. I'd like a test for #pragma GCC visibility push(hidden) which should cause each symbol to be treated as though it were explicitly hidden. Comment at: clang-tidy/fuchsia/AddVisibilityCheck.cpp:32 + if (Name.empty()) return; + Finder->addMatcher(f

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

2018-02-16 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: docs/ReleaseNotes.rst:60 +- New `fuchsia-add-visibility + `_ check Please move into new checks section. Comment at

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

2018-02-16 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang-tidy/fuchsia/AddVisibilityCheck.cpp:32 + if (Name.empty()) return; + Finder->addMatcher(functionDecl(allOf(hasName(Name), isDefinition(), +unless(hasVisibilityAttr( --

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

2018-02-16 Thread Julie Hockett via Phabricator via cfe-commits
juliehockett created this revision. juliehockett added reviewers: aaron.ballman, hokein, alexfh. juliehockett added a project: clang-tools-extra. Herald added subscribers: xazax.hun, mgorny. Adding a checker to find a function given its name and add a visibility attribute if none is present (i.e.