aaron.ballman added inline comments.
================
Comment at: clang-tidy/fuchsia/AddVisibilityCheck.cpp:52-53
+ Finder->addMatcher(
+ functionDecl(allOf(hasAnyName(SmallVector<StringRef, 5>(Names.begin(),
+ Names.end())),
+ isDefinition(), unless(hasVisibilityAttr(V))))
----------------
juliehockett wrote:
> aaron.ballman wrote:
> > Can you use `makeArrayRef()` rather than using a `SmallVector` that
> > allocates?
> Type-wise it gets funky -- `makeArrayRef()` creates an
> `ArrayRef<std::string>`, and the matcher wants a container of `StringRefs`.
> Is there a good way to do that without allocating?
I think there is, but it is orthogonal to your patch to make that many changes,
so you can ignore my suggestion. I didn't see the type shenanigans there.
================
Comment at: clang-tidy/fuchsia/AddVisibilityCheck.cpp:40
+ HeaderFileExtensions, ',')) {
+ llvm::errs() << "Invalid header file extension: "
+ << RawStringHeaderFileExtensions << "\n";
----------------
This works for now, but it would be nice to expose "driver diagnostics" for
sanity checking clang-tidy configuration options. These diagnostics would not
be required to pass in a source location, merely the diagnostic text. We
already support thing kind of things for driver-generated diagnostics, but it's
not exposed to clang-tidy yet.
================
Comment at: clang-tidy/fuchsia/AddVisibilityCheck.cpp:49
+ Vis = DefaultVisibility;
+ else
+ llvm::errs() << "Invalid visibliity attribute: " << VisAttr << "\n";
----------------
jakehehrlich wrote:
> What about internal?
Might be a good use of `StringSwitch`.
================
Comment at: clang-tidy/fuchsia/AddVisibilityCheck.cpp:75
+ Result.SourceManager->getExpansionLoc(D->getLocStart()))) {
+ // unless that file is a header.
+ if (!utils::isSpellingLocInHeaderFile(
----------------
Capitalization
https://reviews.llvm.org/D43392
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
