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 attribute to any definition (declaration or not) that appears in a
header file.
Also updating tests.
https://reviews.llvm.org/D43392
Files:
clang-tidy/fuchsia/AddVisibilityCheck.cpp
clang-tidy/fuchsia/AddVisibilityCheck.h
clang-tidy/fuchsia/CMakeLists.txt
clang-tidy/fuchsia/FuchsiaTidyModule.cpp
docs/ReleaseNotes.rst
docs/clang-tidy/checks/fuchsia-add-visibility.rst
docs/clang-tidy/checks/list.rst
test/clang-tidy/fuchsia-add-visibility-pragma.hpp
test/clang-tidy/fuchsia-add-visibility.cpp
test/clang-tidy/fuchsia-add-visibility.hpp
Index: test/clang-tidy/fuchsia-add-visibility.hpp
===================================================================
--- /dev/null
+++ test/clang-tidy/fuchsia-add-visibility.hpp
@@ -0,0 +1,48 @@
+// RUN: %check_clang_tidy %s fuchsia-add-visibility %t -- \
+// RUN: -config="{CheckOptions: [{key: fuchsia-add-visibility.Names, value: 'foo;bar;baz;foobar;f;g;h;i'}, \
+// RUN: {key: fuchsia-add-visibility.Visibility, value: 'protected'}]}" \
+// RUN: -header-filter=.* \
+// RUN: -- -std=c++11
+
+void foo();
+// CHECK-MESSAGES: [[@LINE-1]]:1: warning: no explicit visibility attribute set for specified function
+// CHECK-FIXES: __attribute__((visibility("protected")))
+
+void foo();
+// CHECK-MESSAGES: [[@LINE-1]]:1: warning: no explicit visibility attribute set for specified function
+// CHECK-FIXES: __attribute__((visibility("protected")))
+
+__attribute__((visibility("default")))
+void i();
+// CHECK-MESSAGES: [[@LINE-2]]:1: warning: protected visibility attribute not set for specified function
+// CHECK-FIXES: __attribute__((visibility("protected")))
+
+__attribute__((visibility("default")))
+void i();
+// CHECK-MESSAGES: [[@LINE-2]]:1: warning: protected visibility attribute not set for specified function
+// CHECK-FIXES: __attribute__((visibility("protected")))
+
+__attribute__((visibility("protected")))
+void bar() {}
+
+__attribute__((visibility("protected")))
+void baz();
+void baz() {}
+
+__attribute__((visibility("default")))
+void foobar() {}
+// CHECK-MESSAGES: [[@LINE-2]]:1: warning: protected visibility attribute not set for specified function
+// CHECK-FIXES: __attribute__((visibility("protected")))
+
+__attribute__((visibility("protected")))
+void foo(int);
+void foo(double);
+// CHECK-MESSAGES: [[@LINE-1]]:1: warning: no explicit visibility attribute set for specified function
+// CHECK-FIXES: __attribute__((visibility("protected")))
+
+template <typename Ty>
+__attribute__((visibility("protected")))
+void h(Ty);
+
+template <>
+void h<int>(int);
Index: test/clang-tidy/fuchsia-add-visibility.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/fuchsia-add-visibility.cpp
@@ -0,0 +1,10 @@
+// RUN: clang-tidy %s -checks=-*,fuchsia-add-visibility \
+// RUN: -config="{CheckOptions: [{key: fuchsia-add-visibility.Names, value: 'foo;bar;baz;foobar;f;g'}, \
+// RUN: {key: fuchsia-add-visibility.Visibility, value: 'protected'}]}" \
+// RUN: -- -std=c++11 | count 0
+
+// Note: this test expects no diagnostics, but FileCheck cannot handle that,
+// hence the use of | count 0.
+
+void foo();
+void foo() {}
Index: test/clang-tidy/fuchsia-add-visibility-pragma.hpp
===================================================================
--- /dev/null
+++ test/clang-tidy/fuchsia-add-visibility-pragma.hpp
@@ -0,0 +1,36 @@
+// RUN: %check_clang_tidy %s fuchsia-add-visibility %t -- \
+// RUN: -config="{CheckOptions: [{key: fuchsia-add-visibility.Names, value: 'foo;bar;baz;foobar;f;g'}, \
+// RUN: {key: fuchsia-add-visibility.Visibility, value: 'protected'}]}" \
+// RUN: -header-filter=.* \
+// RUN: -- -std=c++11
+
+#pragma GCC visibility push(hidden)
+
+void foo();
+// CHECK-MESSAGES: [[@LINE-1]]:1: warning: protected visibility attribute not set for specified function
+// CHECK-FIXES: __attribute__((visibility("protected")))
+
+__attribute__((visibility("protected")))
+void bar() {}
+
+__attribute__((visibility("protected")))
+void baz();
+void baz() {}
+
+__attribute__((visibility("default")))
+void foobar() {}
+// CHECK-MESSAGES: [[@LINE-2]]:1: warning: protected visibility attribute not set for specified function
+// CHECK-FIXES: __attribute__((visibility("protected")))
+
+__attribute__((visibility("protected")))
+void foo(int);
+void foo(double);
+// CHECK-MESSAGES: [[@LINE-1]]:1: warning: protected visibility attribute not set for specified function
+// CHECK-FIXES: __attribute__((visibility("protected")))
+
+template <typename Ty>
+__attribute__((visibility("protected")))
+void foo(Ty);
+
+template <>
+void foo<int>(int);
Index: docs/clang-tidy/checks/list.rst
===================================================================
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -71,6 +71,7 @@
cppcoreguidelines-pro-type-vararg
cppcoreguidelines-slicing
cppcoreguidelines-special-member-functions
+ fuchsia-add-visibility
fuchsia-default-arguments
fuchsia-multiple-inheritance
fuchsia-overloaded-operator
Index: docs/clang-tidy/checks/fuchsia-add-visibility.rst
===================================================================
--- /dev/null
+++ docs/clang-tidy/checks/fuchsia-add-visibility.rst
@@ -0,0 +1,45 @@
+.. title:: clang-tidy - fuchsia-add-visibility
+
+fuchsia-add-visibility
+======================
+
+Finds functions given a list of names and suggests a fix to add
+an `__attribute__((visibility("VISIBILITY")))` to their header declaration,
+if they do not already have a visibility attribute.
+
+Note: this will apply the fix to all declarations/definitions in header
+files, so please use caution before applying it at large.
+
+For example, given the name 'func' in a header file:
+
+.. code-block:: c++
+
+ void func();
+
+would be changed to
+
+.. code-block:: c++
+
+ __attribute__((visibility("hidden")))
+ void func();
+
+Options
+-------
+
+.. option:: Names
+
+ A string containing a comma-separated list of function names to check for
+ a visibility attribute. Default is an empty string.
+
+.. option:: Visibility
+
+ A string specifying what visibility attribute to set, `hidden`, `default`,
+ or `protected`. Default is `hidden`.
+
+.. option:: HeaderFileExtensions
+
+ A comma-separated list of filename extensions of header files (the filename
+ extensions should not contain "." prefix). Default is "h".
+ For header files without an extension, use an empty string (if there are no
+ other desired extensions) or leave an empty element in the list. e.g.,
+ "h,hh,hpp,hxx," (note the trailing comma).
Index: docs/ReleaseNotes.rst
===================================================================
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -70,6 +70,13 @@
with looping constructs. Every backward jump is rejected. Forward jumps are
only allowed in nested loops.
+- New `fuchsia-add-visibility
+ <http://clang.llvm.org/extra/clang-tidy/checks/fuchsia-add-visibility.html>`_ check
+
+ Finds functions given a list of names and suggests a fix to add
+ an `__attribute__((visibility("VISIBILITY")))` to their definitions,
+ if they do not already have a visibility attribute.
+
- New `fuchsia-multiple-inheritance
<http://clang.llvm.org/extra/clang-tidy/checks/fuchsia-multiple-inheritance.html>`_ check
Index: clang-tidy/fuchsia/FuchsiaTidyModule.cpp
===================================================================
--- clang-tidy/fuchsia/FuchsiaTidyModule.cpp
+++ clang-tidy/fuchsia/FuchsiaTidyModule.cpp
@@ -10,6 +10,7 @@
#include "../ClangTidy.h"
#include "../ClangTidyModule.h"
#include "../ClangTidyModuleRegistry.h"
+#include "AddVisibilityCheck.h"
#include "DefaultArgumentsCheck.h"
#include "MultipleInheritanceCheck.h"
#include "OverloadedOperatorCheck.h"
@@ -27,6 +28,8 @@
class FuchsiaModule : public ClangTidyModule {
public:
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
+ CheckFactories.registerCheck<AddVisibilityCheck>(
+ "fuchsia-add-visibility");
CheckFactories.registerCheck<DefaultArgumentsCheck>(
"fuchsia-default-arguments");
CheckFactories.registerCheck<MultipleInheritanceCheck>(
Index: clang-tidy/fuchsia/CMakeLists.txt
===================================================================
--- clang-tidy/fuchsia/CMakeLists.txt
+++ clang-tidy/fuchsia/CMakeLists.txt
@@ -1,6 +1,7 @@
set(LLVM_LINK_COMPONENTS support)
add_clang_library(clangTidyFuchsiaModule
+ AddVisibilityCheck.cpp
DefaultArgumentsCheck.cpp
FuchsiaTidyModule.cpp
MultipleInheritanceCheck.cpp
Index: clang-tidy/fuchsia/AddVisibilityCheck.h
===================================================================
--- /dev/null
+++ clang-tidy/fuchsia/AddVisibilityCheck.h
@@ -0,0 +1,45 @@
+//===--- AddVisibilityCheck.h - clang-tidy-----------------------*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_FUCHSIA_ADDVISIBILITYCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_FUCHSIA_ADDVISIBILITYCHECK_H
+
+#include "../ClangTidy.h"
+#include "../utils/HeaderFileExtensionsUtils.h"
+
+namespace clang {
+namespace tidy {
+namespace fuchsia {
+
+/// Finds functions given a list of names and suggests a fix to add
+/// an `__attribute__((visibility("VISIBILITY")))` to their definitions,
+/// if they do not already have a visibility attribute.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/fuchsia-add-visibility.html
+class AddVisibilityCheck : public ClangTidyCheck {
+ public:
+ AddVisibilityCheck(StringRef Name, ClangTidyContext *Context);
+ void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+ private:
+ std::vector<std::string> Names;
+ const std::string VisAttr;
+ Visibility Vis;
+ const std::string RawStringHeaderFileExtensions;
+ utils::HeaderFileExtensionsSet HeaderFileExtensions;
+};
+
+} // namespace fuchsia
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_FUCHSIA_ADDVISIBILITYCHECK_H
Index: clang-tidy/fuchsia/AddVisibilityCheck.cpp
===================================================================
--- /dev/null
+++ clang-tidy/fuchsia/AddVisibilityCheck.cpp
@@ -0,0 +1,98 @@
+//===--- AddVisibilityCheck.cpp - clang-tidy-------------------------------===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "AddVisibilityCheck.h"
+#include "../utils/OptionsUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Basic/Visibility.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace fuchsia {
+
+AST_MATCHER_P(FunctionDecl, hasVisibilityAttr, Visibility, V) {
+ return Node.getExplicitVisibility(NamedDecl::VisibilityForType) == V;
+}
+
+// AST_MATCHER(FunctionDecl, isInHeaderFile) {
+// return Node.getExplicitVisibility(NamedDecl::VisibilityForType);
+// }
+
+AddVisibilityCheck::AddVisibilityCheck(StringRef Name,
+ ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context),
+ Names(utils::options::parseStringList(
+ Options.get("Names", "::std::vector"))),
+ VisAttr(Options.get("Visibility", "hidden")),
+ RawStringHeaderFileExtensions(Options.getLocalOrGlobal(
+ "HeaderFileExtensions", utils::defaultHeaderFileExtensions())) {
+ if (!utils::parseHeaderFileExtensions(RawStringHeaderFileExtensions,
+ HeaderFileExtensions, ',')) {
+ llvm::errs() << "Invalid header file extension: "
+ << RawStringHeaderFileExtensions << "\n";
+ }
+ if (VisAttr == "hidden")
+ Vis = HiddenVisibility;
+ else if (VisAttr == "protected")
+ Vis = ProtectedVisibility;
+ else if (VisAttr == "default")
+ Vis = DefaultVisibility;
+ else
+ llvm::errs() << "Invalid visibliity attribute: " << VisAttr << "\n";
+}
+
+void AddVisibilityCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+ Options.store(Opts, "HeaderFileExtensions", RawStringHeaderFileExtensions);
+ Options.store(Opts, "Names", utils::options::serializeStringList(Names));
+ Options.store(Opts, "Visibility", VisAttr);
+}
+
+void AddVisibilityCheck::registerMatchers(MatchFinder *Finder) {
+
+ Finder->addMatcher(
+ functionDecl(allOf(hasAnyName(SmallVector<StringRef, 5>(Names.begin(),
+ Names.end())),
+ unless(hasVisibilityAttr(Vis))))
+ .bind("no-visibility"),
+ this);
+}
+
+void AddVisibilityCheck::check(const MatchFinder::MatchResult &Result) {
+ if (const auto *D = Result.Nodes.getNodeAs<FunctionDecl>("no-visibility")) {
+
+ // Ignore if it comes from the "main" file ...
+ if (Result.SourceManager->isInMainFile(
+ Result.SourceManager->getExpansionLoc(D->getLocStart()))) {
+ // unless that file is a header.
+ if (!utils::isSpellingLocInHeaderFile(
+ D->getLocStart(), *Result.SourceManager, HeaderFileExtensions))
+ return;
+ }
+
+ const Attr *A = D->getAttr<VisibilityAttr>();
+ if (A && !A->isInherited() && A->getLocation().isValid())
+ diag(D->getLocStart(),
+ "%0 visibility attribute not set for specified function.")
+ << VisAttr
+ << FixItHint::CreateReplacement(A->getRange(),
+ "visibility(\"" + VisAttr + "\")");
+ else
+ diag(D->getLocStart(),
+ "no explicit visibility attribute set for specified function")
+ << FixItHint::CreateInsertion(D->getLocStart(),
+ "__attribute__((visibility(\"" + VisAttr + "\")))\n");
+ }
+}
+
+} // namespace fuchsia
+} // namespace tidy
+} // namespace clang
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
