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.
================
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))))
----------------
Can you use `makeArrayRef()` rather than using a `SmallVector` that allocates?
================
Comment at: clang-tidy/fuchsia/AddVisibilityCheck.cpp:62
+ Result.Nodes.getNodeAs<FunctionDecl>("no-visibility")) {
+ const Attr *A = D->getAttr<VisibilityAttr>();
+ if (A && !A->isInherited() && A->getLocation().isValid())
----------------
`const auto *`
================
Comment at: clang-tidy/fuchsia/AddVisibilityCheck.cpp:66
+ "%0 visibility attribute not set for specified function")
+ << VisAttr << D
+ << FixItHint::CreateReplacement(A->getRange(),
----------------
Why are you passing `D` as well?
================
Comment at: clang-tidy/fuchsia/AddVisibilityCheck.cpp:72
+ "no explicit visibility attribute set for specified function")
+ << VisAttr << D
+ << FixItHint::CreateInsertion(D->getLocStart(),
----------------
Why are you passing in both `VisAttr` and `D`?
================
Comment at: test/clang-tidy/fuchsia-add-visibility.cpp:25
+// CHECK-MESSAGES: [[@LINE-2]]:1: warning: default visibility attribute not
set for specified function
+// CHECK-FIXES: __attribute__((visibility("default")))
----------------
Should this test case diagnose, assuming `foo` is in the list of functions to
check?
```
__attribute__((visibility("default"))) void foo(int);
void foo(double); // Diagnose?
```
How about this case?
```
template <typename Ty>
__attribute__((visibility("default"))) void foo(Ty);
template <>
void foo<int>(int); // Diagnose?
```
https://reviews.llvm.org/D43392
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
