[clang-tidy] Add "portability" module and rename readability-simd-intrinsics to portability-simd-intrinsics

[MaskRay]

.rst

[MaskRay]

index.rst

[MaskRay]

comment

[MaskRay]

.

[lebedev.ri]

I should have seen in during the initial review, but better now than never, right? :)

clang-tidy/portability/SIMDIntrinsicsCheck.cpp

140

Here too, surely some sane static size can be picked?

clang-tidy/portability/SIMDIntrinsicsCheck.h

32

This could be llvm::SmallString<32> Std;, actually. (or strlen("std::experimental"))

[MaskRay]

std::string -> llvm::SmallString

[MaskRay]

clang-tidy/portability/SIMDIntrinsicsCheck.cpp

140

Changed to llvm::SmallString<32> Std;

The choices can be:

"std::experimental"
"std"
"dimsum" (https://github.com/google/dimsum)

strlen("std::experimental") = 17 is the longest. Raise to 32 because the <32u> template instantiation has been used elsewhere.

clang-tidy/portability/SIMDIntrinsicsCheck.h

32

Answered above.

[Eugene.Zelenko]

Please mention new module in Release Notes. I think new modules should be before new checks there.

[MaskRay]

Mention new module portability in docs/ReleaseNotes.rst

[Eugene.Zelenko]

docs/ReleaseNotes.rst

60

Module and check from it should be mentioned separately. See Release Notes for previous release.

[alexfh]

Thank you!
LGTM

[MaskRay]

ReleaseNotes.rst

clang-tidy/CMakeLists.txt

 add_subdirectory(hicpp)
 add_subdirectory(llvm)
 add_subdirectory(misc)
 add_subdirectory(modernize)
 add_subdirectory(mpi)
 add_subdirectory(objc)
 add_subdirectory(performance)
 add_subdirectory(plugin)
+add_subdirectory(portability)
 add_subdirectory(readability)
 add_subdirectory(tool)
 add_subdirectory(utils)

clang-tidy/plugin/CMakeLists.txt

add_clang_library(clangTidyPlugin
   clangTidyCppCoreGuidelinesModule
   clangTidyGoogleModule
   clangTidyLLVMModule
   clangTidyMiscModule
   clangTidyModernizeModule
   clangTidyMPIModule
   clangTidyObjCModule
   clangTidyPerformanceModule
+  clangTidyPortabilityModule
   clangTidyReadabilityModule
   clangTooling
   )

clang-tidy/plugin/ClangTidyPlugin.cpp

 static int LLVM_ATTRIBUTE_UNUSED MPIModuleAnchorDestination =
   MPIModuleAnchorSource;
 
 // This anchor is used to force the linker to link the PerformanceModule.
 extern volatile int PerformanceModuleAnchorSource;
 static int LLVM_ATTRIBUTE_UNUSED PerformanceModuleAnchorDestination =
     PerformanceModuleAnchorSource;
 
+// This anchor is used to force the linker to link the PortabilityModule.
+extern volatile int PortabilityModuleAnchorSource;
+static int LLVM_ATTRIBUTE_UNUSED PortabilityModuleAnchorDestination =
+    PortabilityModuleAnchorSource;
+
 // This anchor is used to force the linker to link the ReadabilityModule.
 extern volatile int ReadabilityModuleAnchorSource;
 static int LLVM_ATTRIBUTE_UNUSED ReadabilityModuleAnchorDestination =
     ReadabilityModuleAnchorSource;
 
 // This anchor is used to force the linker to link the ObjCModule.
 extern volatile int ObjCModuleAnchorSource;
 static int LLVM_ATTRIBUTE_UNUSED ObjCModuleAnchorDestination =
     ObjCModuleAnchorSource;
 
 } // namespace tidy
 } // namespace clang

clang-tidy/portability/CMakeLists.txt

+set(LLVM_LINK_COMPONENTS support)
+
+add_clang_library(clangTidyPortabilityModule
+  PortabilityTidyModule.cpp
+  SIMDIntrinsicsCheck.cpp
+
+  LINK_LIBS
+  clangAST
+  clangASTMatchers
+  clangBasic
+  clangLex
+  clangTidy
+  clangTidyUtils
+  clangTooling
+  )

clang-tidy/portability/PortabilityTidyModule.cpp

+//===--- PortabilityTidyModule.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 "../ClangTidy.h"
+#include "../ClangTidyModule.h"
+#include "../ClangTidyModuleRegistry.h"
+#include "SIMDIntrinsicsCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace portability {
+
+class PortabilityModule : public ClangTidyModule {
+public:
+  void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
+    CheckFactories.registerCheck<SIMDIntrinsicsCheck>(
+        "portability-simd-intrinsics");
+  }
+};
+
+// Register the PortabilityModule using this statically initialized variable.
+static ClangTidyModuleRegistry::Add<PortabilityModule>
+    X("portability-module", "Adds portability-related checks.");
+
+} // namespace portability
+
+// This anchor is used to force the linker to link in the generated object file
+// and thus register the PortabilityModule.
+volatile int PortabilityModuleAnchorSource = 0;
+
+} // namespace tidy
+} // namespace clang

clang-tidy/portability/SIMDIntrinsicsCheck.h

 //===--- SIMDIntrinsicsCheck.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_SIMD_INTRINSICS_CHECK_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_SIMD_INTRINSICS_CHECK_H
 
 #include "../ClangTidy.h"
 
 namespace clang {
 namespace tidy {
-namespace readability {
+namespace portability {
 
 /// Find SIMD intrinsics calls and suggest std::experimental::simd alternatives.
 ///
 /// For the user-facing documentation see:
-/// http://clang.llvm.org/extra/clang-tidy/checks/readability-simd-intrinsics.html
+/// http://clang.llvm.org/extra/clang-tidy/checks/portability-simd-intrinsics.html
 class SIMDIntrinsicsCheck : public ClangTidyCheck {
 public:
   SIMDIntrinsicsCheck(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::string Std;
   const bool Suggest;
-  StringRef Std;
 };
 
-} // namespace readability
+} // namespace portability
 } // namespace tidy
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_SIMD_INTRINSICS_CHECK_H

clang-tidy/portability/SIMDIntrinsicsCheck.cpp

 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/Triple.h"
 #include "llvm/Support/Regex.h"
 
 using namespace clang::ast_matchers;
 
 namespace clang {
 namespace tidy {
-namespace readability {
+namespace portability {
 
 namespace {
 
 // If the callee has parameter of VectorType or pointer to VectorType,
 // or the return type is VectorType, we consider it a vector function
 // and a candidate for checking.
 AST_MATCHER(FunctionDecl, isVectorFunction) {
   bool IsVector = Node.getReturnType()->isVectorType();
static StringRef TrySuggestX86(StringRef Name) {
   if (Name.startswith("mul_"))
     return "operator* on $simd objects";
 
   return {};
 }
 
 SIMDIntrinsicsCheck::SIMDIntrinsicsCheck(StringRef Name,
                                          ClangTidyContext *Context)
-    : ClangTidyCheck(Name, Context), Suggest(Options.get("Suggest", 0) != 0) {}
+    : ClangTidyCheck(Name, Context), Std(Options.get("Std", "")),
+      Suggest(Options.get("Suggest", 0) != 0) {}
 
 void SIMDIntrinsicsCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "Std", "");
   Options.store(Opts, "Suggest", 0);
 }
 
 void SIMDIntrinsicsCheck::registerMatchers(MatchFinder *Finder) {
   if (!getLangOpts().CPlusPlus11)
     return;
+  // If Std is not specified, infer it from the language options.
   // libcxx implementation backports it to C++11 std::experimental::simd.
+  if (Std.empty())
     Std = getLangOpts().CPlusPlus2a ? "std" : "std::experimental";
 
   Finder->addMatcher(callExpr(callee(functionDecl(allOf(
                                   matchesName("^::(_mm_|_mm256_|_mm512_|vec_)"),
                                   isVectorFunction()))),
                               unless(isExpansionInSystemHeader()))
                          .bind("call"),
                      this);
 }
 
 void SIMDIntrinsicsCheck::check(const MatchFinder::MatchResult &Result) {
   const auto *Call = Result.Nodes.getNodeAs<CallExpr>("call");
   assert(Call != nullptr);
   const FunctionDecl *Callee = Call->getDirectCallee();
   if (!Callee)
     return;
 
   StringRef Old = Callee->getName();
   StringRef New;
   llvm::Triple::ArchType Arch =
       Result.Context->getTargetInfo().getTriple().getArch();
 
+  // We warn or suggest if this SIMD intrinsic function has a std::simd
+  // replacement.
   switch (Arch) {
-    default:
-      break;
-    case llvm::Triple::ppc:
-    case llvm::Triple::ppc64:
-    case llvm::Triple::ppc64le:
-      New = TrySuggestPPC(Old);
-      break;
-    case llvm::Triple::x86:
-    case llvm::Triple::x86_64:
-      New = TrySuggestX86(Old);
-      break;
   }
 
   if (!New.empty()) {
     std::string Message;
     // If Suggest is true, give a P0214 alternative, otherwise point it out it
     // is non-portable.
     if (Suggest) {
       Message = (Twine("'") + Old + "' can be replaced by " + New).str();
       Message = llvm::Regex("\\$std").sub(Std, Message);
-      Message = llvm::Regex("\\$simd").sub(Std.str() + "::simd", Message);
+      Message = llvm::Regex("\\$simd").sub(Std + "::simd", Message);
     } else {
       Message = (Twine("'") + Old + "' is a non-portable " +
                  llvm::Triple::getArchTypeName(Arch) + " intrinsic function")
                     .str();
     }
     diag(Call->getExprLoc(), Message);
   }
 }
 
-} // namespace readability
+} // namespace portability
 } // namespace tidy
 } // namespace clang

clang-tidy/readability/CMakeLists.txt

add_clang_library(clangTidyReadabilityModule
   ReadabilityTidyModule.cpp
   RedundantControlFlowCheck.cpp
   RedundantDeclarationCheck.cpp
   RedundantFunctionPtrDereferenceCheck.cpp
   RedundantMemberInitCheck.cpp
   RedundantStringCStrCheck.cpp
   RedundantSmartptrGetCheck.cpp
   RedundantStringInitCheck.cpp
-  SIMDIntrinsicsCheck.cpp
   SimplifyBooleanExprCheck.cpp
   StaticAccessedThroughInstanceCheck.cpp
   StaticDefinitionInAnonymousNamespaceCheck.cpp
   StringCompareCheck.cpp
   UniqueptrDeleteReleaseCheck.cpp
 
   LINK_LIBS
   clangAST
   clangASTMatchers
   clangBasic
   clangLex
   clangTidy
   clangTidyUtils
   clangTooling
   )

clang-tidy/readability/ReadabilityTidyModule.cpp

 #include "NonConstParameterCheck.h"
 #include "RedundantControlFlowCheck.h"
 #include "RedundantDeclarationCheck.h"
 #include "RedundantFunctionPtrDereferenceCheck.h"
 #include "RedundantMemberInitCheck.h"
 #include "RedundantSmartptrGetCheck.h"
 #include "RedundantStringCStrCheck.h"
 #include "RedundantStringInitCheck.h"
-#include "SIMDIntrinsicsCheck.h"
 #include "SimplifyBooleanExprCheck.h"
 #include "StaticAccessedThroughInstanceCheck.h"
 #include "StaticDefinitionInAnonymousNamespaceCheck.h"
 #include "StringCompareCheck.h"
 #include "UniqueptrDeleteReleaseCheck.h"
 
 namespace clang {
 namespace tidy {
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
     CheckFactories.registerCheck<RedundantDeclarationCheck>(
         "readability-redundant-declaration");
     CheckFactories.registerCheck<RedundantSmartptrGetCheck>(
         "readability-redundant-smartptr-get");
     CheckFactories.registerCheck<RedundantStringCStrCheck>(
         "readability-redundant-string-cstr");
     CheckFactories.registerCheck<RedundantStringInitCheck>(
         "readability-redundant-string-init");
-    CheckFactories.registerCheck<SIMDIntrinsicsCheck>(
-        "readability-simd-intrinsics");
     CheckFactories.registerCheck<SimplifyBooleanExprCheck>(
         "readability-simplify-boolean-expr");
     CheckFactories.registerCheck<UniqueptrDeleteReleaseCheck>(
         "readability-uniqueptr-delete-release");
   }
 };
 
 // Register the ReadabilityModule using this statically initialized variable.

clang-tidy/tool/CMakeLists.txt

target_link_libraries(clang-tidy
   clangTidyGoogleModule
   clangTidyHICPPModule
   clangTidyLLVMModule
   clangTidyMiscModule
   clangTidyModernizeModule
   clangTidyMPIModule
   clangTidyObjCModule
   clangTidyPerformanceModule
+  clangTidyPortabilityModule
   clangTidyReadabilityModule
   clangTooling
   clangToolingCore
   )
 
 install(PROGRAMS clang-tidy-diff.py
   DESTINATION share/clang
   COMPONENT clang-tidy)
 install(PROGRAMS run-clang-tidy.py
   DESTINATION share/clang
   COMPONENT clang-tidy)

clang-tidy/tool/ClangTidyMain.cpp

 static int LLVM_ATTRIBUTE_UNUSED MPIModuleAnchorDestination =
     MPIModuleAnchorSource;
 
 // This anchor is used to force the linker to link the PerformanceModule.
 extern volatile int PerformanceModuleAnchorSource;
 static int LLVM_ATTRIBUTE_UNUSED PerformanceModuleAnchorDestination =
     PerformanceModuleAnchorSource;
 
+// This anchor is used to force the linker to link the PortabilityModule.
+extern volatile int PortabilityModuleAnchorSource;
+static int LLVM_ATTRIBUTE_UNUSED PortabilityModuleAnchorDestination =
+    PortabilityModuleAnchorSource;
+
 // This anchor is used to force the linker to link the ReadabilityModule.
 extern volatile int ReadabilityModuleAnchorSource;
 static int LLVM_ATTRIBUTE_UNUSED ReadabilityModuleAnchorDestination =
     ReadabilityModuleAnchorSource;
 
 // This anchor is used to force the linker to link the ObjCModule.
 extern volatile int ObjCModuleAnchorSource;
 static int LLVM_ATTRIBUTE_UNUSED ObjCModuleAnchorDestination =

docs/ReleaseNotes.rst

 Improvements to clang-rename
 ----------------------------
 
 The improvements are...
 
 Improvements to clang-tidy
 --------------------------
 
 - New `bugprone-throw-keyword-missing
   <http://clang.llvm.org/extra/clang-tidy/checks/bugprone-throw-keyword-missing.html>`_ check
 
   Diagnoses when a temporary object that appears to be an exception is
   constructed but not thrown.
 
 - New `cppcoreguidelines-avoid-goto
   <http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-avoid-goto.html>`_ check
 
- New `fuchsia-trailing-return
   return types.
 
 - New `modernize-use-uncaught-exceptions
   <http://clang.llvm.org/extra/clang-tidy/checks/modernize-use-uncaught-exceptions.html>`_ check
 
   Finds and replaces deprecated uses of ``std::uncaught_exception`` to
   ``std::uncaught_exceptions``.
 
-- New `readability-simd-intrinsics
-  <http://clang.llvm.org/extra/clang-tidy/checks/readability-simd-intrinsics.html>`_ check
+- New `portability-simd-intrinsics
+  <http://clang.llvm.org/extra/clang-tidy/checks/portability-simd-intrinsics.html>`_ check
 
   Warns if SIMD intrinsics are used which can be replaced by
   ``std::experimental::simd`` operations.
 
 - New alias `hicpp-avoid-goto
   <http://clang.llvm.org/extra/clang-tidy/checks/hicpp-avoid-goto.html>`_ to
   `cppcoreguidelines-avoid-goto <http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-avoid-goto.html>`_
   added.

docs/clang-tidy/checks/portability-simd-intrinsics.rst

-.. title:: clang-tidy - readability-simd-intrinsics
+.. title:: clang-tidy - portability-simd-intrinsics
 
-readability-simd-intrinsics
+portability-simd-intrinsics
 ===========================
 
 Finds SIMD intrinsics calls and suggests ``std::experimental::simd`` (`P0214`_)
 alternatives.
 
 If the option ``Suggest`` is set to non-zero, for
 
 .. code-block:: c++

 -------
 
 .. option:: Suggest
 
    If this option is set to non-zero (default is `0`), the check will suggest
    `P0214`_ alternatives, otherwise it only points out the intrinsic function is
    non-portable.
 
+.. option:: Std
+
+   The namespace used to suggest `P0214`_ alternatives. If not specified, `std::`
+   for `-std=c++2a` and `std::experimental::` for `-std=c++11`.
+
 .. _P0214: http://wg21.link/p0214

docs/clang-tidy/index.rst

 ``hicpp-``             Checks related to High Integrity C++ Coding Standard.
 ``llvm-``              Checks related to the LLVM coding conventions.
 ``misc-``              Checks that we didn't have a better category for.
 ``modernize-``         Checks that advocate usage of modern (currently "modern"
                        means "C++11") language constructs.
 ``mpi-``               Checks related to MPI (Message Passing Interface).
 ``objc-``              Checks related to Objective-C coding conventions.
 ``performance-``       Checks that target performance-related issues.
+``portability-``       Checks that target portability-related issues that don't
+                       relate to any particular coding style.
 ``readability-``       Checks that target readability-related issues that don't
                        relate to any particular coding style.
 ====================== =========================================================
 
 Clang diagnostics are treated in a similar way as check diagnostics. Clang
 diagnostics are displayed by :program:`clang-tidy` and can be filtered out using
 ``-checks=`` option. However, the ``-checks=`` option does not affect
 compilation arguments, so it can not turn on Clang warnings which are not

test/clang-tidy/portability-simd-intrinsics-ppc.cpp

-// RUN: %check_clang_tidy %s readability-simd-intrinsics %t -- \
+// RUN: %check_clang_tidy %s portability-simd-intrinsics %t -- \
 // RUN:  -config='{CheckOptions: [ \
-// RUN:    {key: readability-simd-intrinsics.Suggest, value: 1} \
+// RUN:    {key: portability-simd-intrinsics.Suggest, value: 1} \
 // RUN:  ]}' -- -target ppc64le -maltivec -std=c++11
 
 vector int vec_add(vector int, vector int);
 
 void PPC() {
   vector int i0, i1;
 
   vec_add(i0, i1);
-// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'vec_add' can be replaced by operator+ on std::experimental::simd objects [readability-simd-intrinsics]
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'vec_add' can be replaced by operator+ on std::experimental::simd objects [portability-simd-intrinsics]
 }

test/clang-tidy/portability-simd-intrinsics-x86.cpp

-// RUN: %check_clang_tidy %s readability-simd-intrinsics %t -- \
+// RUN: %check_clang_tidy %s portability-simd-intrinsics %t -- \
 // RUN:  -config='{CheckOptions: [ \
-// RUN:    {key: readability-simd-intrinsics.Suggest, value: 1} \
+// RUN:    {key: portability-simd-intrinsics.Suggest, value: 1} \
 // RUN:  ]}' -- -target x86_64 -std=c++11
 
 typedef long long __m128i __attribute__((vector_size(16)));
 typedef double __m256 __attribute__((vector_size(32)));
 
 __m128i _mm_add_epi32(__m128i, __m128i);
 __m256 _mm256_load_pd(double const *);
 void _mm256_store_pd(double *, __m256);
 
 int _mm_add_fake(int, int);
 
 void X86() {
   __m128i i0, i1;
   __m256 d0;
 
   _mm_add_epi32(i0, i1);
-// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: '_mm_add_epi32' can be replaced by operator+ on std::experimental::simd objects [readability-simd-intrinsics]
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: '_mm_add_epi32' can be replaced by operator+ on std::experimental::simd objects [portability-simd-intrinsics]
   d0 = _mm256_load_pd(0);
   _mm256_store_pd(0, d0);
 
   _mm_add_fake(0, 1);
 }