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 comments and documentation


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.cpp

Index: test/clang-tidy/fuchsia-add-visibility.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/fuchsia-add-visibility.cpp
@@ -0,0 +1,18 @@
+// RUN: %check_clang_tidy %s fuchsia-add-visibility %t -- \
+// RUN:   -config="{CheckOptions: [{key: fuchsia-add-visibility.Names, value: 'foo,bar,baz'}, \
+// RUN:								{key: fuchsia-add-visibility.Visibility, value: 'hidden'}]}" \
+// RUN:   -- -std=c++11
+
+void foo();
+
+void foo() {}
+// CHECK-MESSAGES: [[@LINE-1]]:1: warning: visibility attribute not set for specified function
+// CHECK-FIXES: __attribute__((visibility("hidden")))
+
+__attribute__((visibility("internal")))
+void bar() {}
+
+__attribute__((visibility("protected")))
+void baz();
+
+void baz() {}
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,34 @@
+.. 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 definitions,
+if they do not already have a visibility attribute.
+
+For example, given the name 'func':
+
+.. 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 visibilit attribute. Default is an empty string.
+
+.. option:: Visibility
+
+   A string specifying what visibility attribute to set, `hidden`, `default`,
+   `internal`, or `protected`. Default is `hidden`.
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,44 @@
+//===--- 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 "llvm/ADT/SmallVector.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:
+  void loadNames();
+
+  std::string Visibility;
+  llvm::SmallVector<std::string, 8> Names;
+};
+
+}  // 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,77 @@
+//===--- 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 "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "llvm/Support/ErrorOr.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/raw_ostream.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace fuchsia {
+
+AST_MATCHER(FunctionDecl, hasVisibilityAttr) {
+  if (const auto *D = Node.getDefinition())
+    return D->getLinkageAndVisibility().isVisibilityExplicit();
+  return Node.getLinkageAndVisibility().isVisibilityExplicit();
+}
+
+AddVisibilityCheck::AddVisibilityCheck(StringRef Name,
+                                       ClangTidyContext *Context)
+    : ClangTidyCheck(Name, Context),
+      Visibility(Options.get("Visibility", "hidden")) {
+  // Use a temp StringRef array to store the values of the substrings,
+  // and then copy the string data into the class scope
+  llvm::SmallVector<StringRef, 8> NamesRefs;
+  StringRef(Options.get("Names", "")).split(NamesRefs, ",");
+  for (const auto &Name : NamesRefs) {
+    if (!Name.empty()) Names.push_back(Name);
+  }
+}
+
+void AddVisibilityCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+	// Rebuild string representation of Names vector
+	std::string NamesOut;
+	llvm::raw_string_ostream Stream(NamesOut);
+	if (!Names.empty())
+		Stream << Names[0];
+	for (unsigned i = 1; i < Names.size(); ++i)
+		Stream << "," << Names[i];
+  Options.store(Opts, "Names", NamesOut);
+  Options.store(Opts, "Visibility", Visibility);
+}
+
+void AddVisibilityCheck::registerMatchers(MatchFinder *Finder) {
+  for (const auto &Name : Names)
+    Finder->addMatcher(functionDecl(allOf(hasName(Name), isDefinition(),
+                                          unless(hasVisibilityAttr())))
+                           .bind(Name),
+                       this);
+}
+
+void AddVisibilityCheck::check(const MatchFinder::MatchResult &Result) {
+  for (const auto &Name : Names) {
+    if (const auto *MatchedDecl = Result.Nodes.getNodeAs<FunctionDecl>(Name)) {
+      std::string VisAttr =
+          ("__attribute__((visibility(\"" + Visibility + "\")))\n");
+      diag(MatchedDecl->getLocStart(),
+           "visibility attribute not set for specified function")
+          << MatchedDecl
+          << FixItHint::CreateInsertion(MatchedDecl->getLocStart(), VisAttr);
+    }
+  }
+}
+
+}  // namespace fuchsia
+}  // namespace tidy
+}  // namespace clang
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to