Index: clang-tidy/CMakeLists.txt =================================================================== --- clang-tidy/CMakeLists.txt +++ clang-tidy/CMakeLists.txt @@ -49,6 +49,7 @@ add_subdirectory(mpi) endif() add_subdirectory(objc) +add_subdirectory(openmp) add_subdirectory(performance) add_subdirectory(plugin) add_subdirectory(portability) Index: clang-tidy/ClangTidyForceLinker.h =================================================================== --- clang-tidy/ClangTidyForceLinker.h +++ clang-tidy/ClangTidyForceLinker.h @@ -77,6 +77,11 @@ MPIModuleAnchorSource; #endif +// This anchor is used to force the linker to link the OpenMPModule. +extern volatile int OpenMPModuleAnchorSource; +static int LLVM_ATTRIBUTE_UNUSED OpenMPModuleAnchorDestination = + OpenMPModuleAnchorSource; + // This anchor is used to force the linker to link the PerformanceModule. extern volatile int PerformanceModuleAnchorSource; static int LLVM_ATTRIBUTE_UNUSED PerformanceModuleAnchorDestination = Index: clang-tidy/openmp/CMakeLists.txt =================================================================== --- /dev/null +++ clang-tidy/openmp/CMakeLists.txt @@ -0,0 +1,12 @@ +set(LLVM_LINK_COMPONENTS support) + +add_clang_library(clangTidyOpenMPModule + OpenMPTidyModule.cpp + UseDefaultNoneCheck.cpp + + LINK_LIBS + clangAST + clangASTMatchers + clangBasic + clangTidy + ) Index: clang-tidy/openmp/OpenMPTidyModule.cpp =================================================================== --- /dev/null +++ clang-tidy/openmp/OpenMPTidyModule.cpp @@ -0,0 +1,38 @@ +//===--- OpenMPTidyModule.cpp - clang-tidy--------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "../ClangTidy.h" +#include "../ClangTidyModule.h" +#include "../ClangTidyModuleRegistry.h" +#include "UseDefaultNoneCheck.h" + +namespace clang { +namespace tidy { +namespace openmp { + +/// This module is for OpenMP-specific checks. +class OpenMPModule : public ClangTidyModule { +public: + void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override { + CheckFactories.registerCheck( + "openmp-use-default-none"); + } +}; + +// Register the OpenMPTidyModule using this statically initialized variable. +static ClangTidyModuleRegistry::Add + X("openmp-module", "Adds OpenMP-specific checks."); + +} // namespace openmp + +// This anchor is used to force the linker to link in the generated object file +// and thus register the OpenMPModule. +volatile int OpenMPModuleAnchorSource = 0; + +} // namespace tidy +} // namespace clang Index: clang-tidy/openmp/UseDefaultNoneCheck.h =================================================================== --- /dev/null +++ clang-tidy/openmp/UseDefaultNoneCheck.h @@ -0,0 +1,36 @@ +//===--- UseDefaultNoneCheck.h - clang-tidy ---------------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_OPENMP_USEDEFAULTNONECHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_OPENMP_USEDEFAULTNONECHECK_H + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { +namespace openmp { + +/// Finds OpenMP directives that are allowed to contain ``default`` clause, +/// but either don``t specify it, or the clause is specified but with the kind +/// other than ``none``, and suggests to use ``default(none)`` clause. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/openmp-use-default-none.html +class UseDefaultNoneCheck : public ClangTidyCheck { +public: + UseDefaultNoneCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; +}; + +} // namespace openmp +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_OPENMP_USEDEFAULTNONECHECK_H Index: clang-tidy/openmp/UseDefaultNoneCheck.cpp =================================================================== --- /dev/null +++ clang-tidy/openmp/UseDefaultNoneCheck.cpp @@ -0,0 +1,155 @@ +//===--- UseDefaultNoneCheck.cpp - clang-tidy -----------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "UseDefaultNoneCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/AST/OpenMPClause.h" +#include "clang/AST/Stmt.h" +#include "clang/AST/StmtOpenMP.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/ASTMatchers/ASTMatchersMacros.h" + +using namespace clang::ast_matchers; + +namespace clang { + +namespace { + +/// Matches any '#pragma omp' executable directive. +/// +/// Example: +/// +/// \code +/// #pragma omp parallel +/// #pragma omp for <...> +/// \endcode +const ast_matchers::internal::VariadicDynCastAllOfMatcher< + Stmt, OMPExecutableDirective> + ompExecutableDirective; + +/// Matches if the OpenMP directive is allowed to contain the specified OpenMP +/// clause kind. +/// +/// Example: +/// +/// Given, `ompExecutableDirective(isAllowedToContainClause(OMPC_default))`: +/// +/// \code +/// #pragma omp parallel // <- match +/// #pragma omp parallel for // <- match +/// #pragma omp for // <- no match +/// \endcode +/// +/// FIXME: would be better to pass the actual class name (e.g. OMPDefaultClause) +/// instead of the actual OpenMPClauseKind. +AST_MATCHER_P(OMPExecutableDirective, isAllowedToContainClause, + OpenMPClauseKind, CKind) { + return isAllowedClauseForDirective(Node.getDirectiveKind(), CKind); +} + +/// Matches first matching OpenMP clause that matches InnerMatcher. +/// +/// Example: +/// +/// Given 'ompExecutableDirective(hasClause(anything()))': +/// +/// \code +/// #pragma omp parallel // <- no clauses, no match +/// #pragma omp parallel default(none) // <- has clauses, match +/// \endcode +AST_MATCHER_P(OMPExecutableDirective, hasClause, + ast_matchers::internal::Matcher, InnerMatcher) { + ArrayRef Clauses = Node.clauses(); + return matchesFirstInPointerRange(InnerMatcher, Clauses.begin(), + Clauses.end(), Finder, Builder); +} + +/// Matches OpenMP 'default(...)' clause. +/// +/// Example: +/// +/// \code +/// #pragma omp <...> default(none) // <- will match 'default(none)' +/// #pragma omp <...> default(shared) // <- will match 'default(shared)' +/// #pragma omp <...> // <- no match +/// \endcode +const ast_matchers::internal::VariadicDynCastAllOfMatcher + ompDefaultClause; + +/// Matches the specific default kind in the OpenMP 'default(...)' clause. +/// +/// Example: +/// +/// Given, `ompDefaultClause(hasKind(OMPC_DEFAULT_none))`: +/// +/// \code +/// #pragma omp parallel default(none) // <- match +/// #pragma omp parallel default(shared) // <- no match +/// \endcode +/// +/// Given, `ompDefaultClause(hasKind(OMPC_DEFAULT_shared))`: +/// +/// \code +/// #pragma omp parallel default(none) // <- no match +/// #pragma omp parallel default(shared) // <- match +/// \endcode +AST_MATCHER_P(OMPDefaultClause, hasKind, OpenMPDefaultClauseKind, Kind) { + return Node.getDefaultKind() == Kind; +} + +} // namespace + +namespace tidy { +namespace openmp { + +void UseDefaultNoneCheck::registerMatchers(MatchFinder *Finder) { + // If OpenMP is not enabled, don't register the check, it won't find anything. + if (!getLangOpts().OpenMP) + return; + + Finder->addMatcher( + ompExecutableDirective( + allOf(isAllowedToContainClause(OMPC_default), + anyOf(unless(hasClause(ompDefaultClause())), + hasClause( + ompDefaultClause(unless(hasKind(OMPC_DEFAULT_none))) + .bind("clause"))))) + .bind("directive"), + this); +} + +void UseDefaultNoneCheck::check(const MatchFinder::MatchResult &Result) { + const auto *Directive = + Result.Nodes.getNodeAs("directive"); + assert(Directive != nullptr); + + if (const auto *Clause = Result.Nodes.getNodeAs("clause")) { + diag(Directive->getBeginLoc(), + "OpenMP directive '%0' is allowed to contain the 'default' clause, " + "and 'default' clause exists with '%1' kind. Consider using " + "'default(none)' clause instead.") + << getOpenMPDirectiveName(Directive->getDirectiveKind()) + << getOpenMPSimpleClauseTypeName(Clause->getClauseKind(), + Clause->getDefaultKind()); + diag(Clause->getBeginLoc(), "Existing 'default' clause is specified here.", + DiagnosticIDs::Note); + return; + } + + diag(Directive->getBeginLoc(), + "OpenMP directive '%0' is allowed to contain the 'default' clause, but " + "no 'default' clause is specified. Consider specifying 'default(none)' " + "clause.") + << getOpenMPDirectiveName(Directive->getDirectiveKind()); +} + +} // namespace openmp +} // namespace tidy +} // namespace clang Index: clang-tidy/plugin/CMakeLists.txt =================================================================== --- clang-tidy/plugin/CMakeLists.txt +++ clang-tidy/plugin/CMakeLists.txt @@ -21,6 +21,7 @@ clangTidyMiscModule clangTidyModernizeModule clangTidyObjCModule + clangTidyOpenMPModule clangTidyPerformanceModule clangTidyPortabilityModule clangTidyReadabilityModule Index: clang-tidy/tool/CMakeLists.txt =================================================================== --- clang-tidy/tool/CMakeLists.txt +++ clang-tidy/tool/CMakeLists.txt @@ -30,6 +30,7 @@ clangTidyMiscModule clangTidyModernizeModule clangTidyObjCModule + clangTidyOpenMPModule clangTidyPerformanceModule clangTidyPortabilityModule clangTidyReadabilityModule Index: docs/ReleaseNotes.rst =================================================================== --- docs/ReleaseNotes.rst +++ docs/ReleaseNotes.rst @@ -67,12 +67,23 @@ Improvements to clang-tidy -------------------------- +- New OpenMP module. + + For checks specific to `OpenMP `_ API. + - New :doc:`abseil-duration-conversion-cast ` check. Checks for casts of ``absl::Duration`` conversion functions, and recommends the right conversion function instead. +- New :doc:`openmp-use-default-none + ` check. + + Finds OpenMP directives that are allowed to contain ``default`` clause, + but either don't specify it, or the clause is specified but with the kind + other than ``none``, and suggests to use ``default(none)`` clause. + Improvements to include-fixer ----------------------------- Index: docs/clang-tidy/checks/list.rst =================================================================== --- docs/clang-tidy/checks/list.rst +++ docs/clang-tidy/checks/list.rst @@ -221,6 +221,7 @@ objc-avoid-spinlock objc-forbidden-subclassing objc-property-declaration + openmp-use-default-none performance-faster-string-find performance-for-range-copy performance-implicit-conversion-in-loop Index: docs/clang-tidy/checks/openmp-use-default-none.rst =================================================================== --- /dev/null +++ docs/clang-tidy/checks/openmp-use-default-none.rst @@ -0,0 +1,69 @@ +.. title:: clang-tidy - openmp-use-default-none + +openmp-use-default-none +======================= + +Finds OpenMP directives that are allowed to contain ``default`` clause, +but either don't specify it, or the clause is specified but with the kind +other than ``none``, and suggests to use ``default(none)`` clause. + +Using ``default(none)`` clause changes the default variable visibility from +being implicitly determined, and thus forces developer to be explicit about the +desired data scoping for each variable. + +Example +------- + +.. code-block:: c++ + + // 'for' directive can not have 'default' clause, no diagnostics. + void t0(const int a) { + #pragma omp for + for (int b = 0; b < a; b++) + ; + } + + // 'parallel' directive can have 'default' clause, but said clause is not specified, diagnosed. + void t1() { + #pragma omp parallel // <- warning: OpenMP directive 'parallel' is allowed to contain the 'default' clause, but no 'default' clause is specified. Consider specifying 'default(none)' clause. + ; + } + + // 'parallel' directive can have 'default' clause, and said clause is specified, with 'none' kind, all good. + void t2() { + #pragma omp parallel default(none) + ; + } + + // 'parallel' directive can have 'default' clause, and said clause is specified but with 'shared' kind, which is not 'none', diagnose. + void t3() { + #pragma omp parallel default(shared) // <- warning: OpenMP directive 'parallel' is allowed to contain the 'default' clause, and 'default' clause exists with 'shared' kind. Consider using 'default(none)' clause instead. + // ^ note: Existing 'default' clause is specified here. + ; + } + + //----------------------------------------------------------------------------// + // Combined directives are handled correctly. + //----------------------------------------------------------------------------// + + // 'parallel' directive can have 'default' clause, but said clause is not specified, diagnosed. + void t4(const int a) { + #pragma omp parallel for + for (int b = 0; b < a; b++) // <- warning: OpenMP directive 'parallel for' is allowed to contain the 'default' clause, but no 'default' clause is specified. Consider specifying 'default(none)' clause. + ; + } + + // 'parallel' directive can have 'default' clause, and said clause is specified with 'none' kind, all good. + void t5(const int a) { + #pragma omp parallel for default(none) + for (int b = 0; b < a; b++) + ; + } + + // 'parallel' directive can have 'default' clause, and said clause is specified, but with 'shared' kind, which is not 'none', diagnose. + void t6(const int a) { + #pragma omp parallel for default(shared) // <- warning: OpenMP directive 'parallel for' is allowed to contain the 'default' clause, and 'default' clause exists with 'shared' kind. Consider using 'default(none)' clause instead. + // ^ note: Existing 'default' clause is specified here. + for (int b = 0; b < a; b++) + ; + } Index: test/clang-tidy/openmp-use-default-none.cpp =================================================================== --- /dev/null +++ test/clang-tidy/openmp-use-default-none.cpp @@ -0,0 +1,68 @@ +// RUN: %check_clang_tidy %s openmp-use-default-none %t -- -- -x c++ -fopenmp -fopenmp-version=31 +// RUN: %check_clang_tidy %s openmp-use-default-none %t -- -- -x c -fopenmp -fopenmp-version=31 + +//----------------------------------------------------------------------------// +// Trivial cases. +//----------------------------------------------------------------------------// + +// 'for' directive can not have 'default' clause, no diagnostics. +void t0(const int a) { +#pragma omp for + for (int b = 0; b < a; b++) + ; +} + +// 'parallel' directive can have 'default' clause, but said clause is not +// specified, diagnosed. +void t1() { +#pragma omp parallel + ; + // CHECK-NOTES: :[[@LINE-2]]:9: warning: OpenMP directive 'parallel' is allowed to contain the 'default' clause, but no 'default' clause is specified. Consider specifying 'default(none)' clause. +} + +// 'parallel' directive can have 'default' clause, and said clause is specified, +// with 'none' kind, all good. +void t2() { +#pragma omp parallel default(none) + ; +} + +// 'parallel' directive can have 'default' clause, and said clause is specified, +// but with 'shared' kind, which is not 'none', diagnose. +void t3() { +#pragma omp parallel default(shared) + ; + // CHECK-NOTES: :[[@LINE-2]]:9: warning: OpenMP directive 'parallel' is allowed to contain the 'default' clause, and 'default' clause exists with 'shared' kind. Consider using 'default(none)' clause instead. + // CHECK-NOTES: :[[@LINE-3]]:22: note: Existing 'default' clause is specified here. +} + +//----------------------------------------------------------------------------// +// Two directive cases. +//----------------------------------------------------------------------------// + +// 'parallel' directive can have 'default' clause, but said clause is not +// specified, diagnosed. +void t4(const int a) { +#pragma omp parallel for + for (int b = 0; b < a; b++) + ; + // CHECK-NOTES: :[[@LINE-3]]:9: warning: OpenMP directive 'parallel for' is allowed to contain the 'default' clause, but no 'default' clause is specified. Consider specifying 'default(none)' clause. +} + +// 'parallel' directive can have 'default' clause, and said clause is specified, +// with 'none' kind, all good. +void t5(const int a) { +#pragma omp parallel for default(none) + for (int b = 0; b < a; b++) + ; +} + +// 'parallel' directive can have 'default' clause, and said clause is specified, +// but with 'shared' kind, which is not 'none', diagnose. +void t6(const int a) { +#pragma omp parallel for default(shared) + for (int b = 0; b < a; b++) + ; + // CHECK-NOTES: :[[@LINE-3]]:9: warning: OpenMP directive 'parallel for' is allowed to contain the 'default' clause, and 'default' clause exists with 'shared' kind. Consider using 'default(none)' clause instead. + // CHECK-NOTES: :[[@LINE-4]]:26: note: Existing 'default' clause is specified here. +}