Index: clang-tidy/openmp/CMakeLists.txt =================================================================== --- clang-tidy/openmp/CMakeLists.txt +++ clang-tidy/openmp/CMakeLists.txt @@ -2,6 +2,7 @@ add_clang_library(clangTidyOpenMPModule OpenMPTidyModule.cpp + UseDefaultNoneCheck.cpp LINK_LIBS clangAST Index: clang-tidy/openmp/OpenMPTidyModule.cpp =================================================================== --- clang-tidy/openmp/OpenMPTidyModule.cpp +++ clang-tidy/openmp/OpenMPTidyModule.cpp @@ -9,6 +9,7 @@ #include "../ClangTidy.h" #include "../ClangTidyModule.h" #include "../ClangTidyModuleRegistry.h" +#include "UseDefaultNoneCheck.h" namespace clang { namespace tidy { @@ -18,6 +19,8 @@ class OpenMPModule : public ClangTidyModule { public: void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override { + CheckFactories.registerCheck( + "openmp-use-default-none"); } }; 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 a ``default`` clause, +/// but either don't specify it or the clause is specified but with the kind +/// other than ``none``, and suggests to use the ``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,65 @@ +//===--- 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 tidy { +namespace openmp { + +void UseDefaultNoneCheck::registerMatchers(MatchFinder *Finder) { + // Don't register the check if OpenMP is not enabled; the OpenMP pragmas are + // completely ignored then, so no OpenMP entires will be present in the AST. + if (!getLangOpts().OpenMP) + return; + + Finder->addMatcher( + ompExecutableDirective( + allOf(isAllowedToContainClauseKind(OMPC_default), + anyOf(unless(hasAnyClause(ompDefaultClause())), + hasAnyClause(ompDefaultClause(unless(isNoneKind())) + .bind("clause"))))) + .bind("directive"), + this); +} + +void UseDefaultNoneCheck::check(const MatchFinder::MatchResult &Result) { + const auto *Directive = + Result.Nodes.getNodeAs("directive"); + assert(Directive != nullptr && "Expected to match some directive."); + + if (const auto *Clause = Result.Nodes.getNodeAs("clause")) { + diag(Directive->getBeginLoc(), + "OpenMP directive '%0' specifies 'default(%1)' clause. 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' does not specify 'default' clause. Consider " + "specifying 'default(none)' clause.") + << getOpenMPDirectiveName(Directive->getDirectiveKind()); +} + +} // namespace openmp +} // namespace tidy +} // namespace clang Index: docs/ReleaseNotes.rst =================================================================== --- docs/ReleaseNotes.rst +++ docs/ReleaseNotes.rst @@ -130,6 +130,13 @@ ` now supports `OverrideSpelling` and `FinalSpelling` options. +- New :doc:`openmp-use-default-none + ` check. + + Finds OpenMP directives that are allowed to contain a ``default`` clause, + but either don't specify it or the clause is specified but with the kind + other than ``none``, and suggests to use the ``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 @@ -227,6 +227,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,170 @@ +.. title:: clang-tidy - openmp-use-default-none + +openmp-use-default-none +======================= + +Finds OpenMP directives that are allowed to contain a ``default`` clause, +but either don't specify it or the clause is specified but with the kind +other than ``none``, and suggests to use the ``default(none)`` clause. + +Using ``default(none)`` clause forces developers to explicitly specify data +sharing attributes for the variables referenced in the construct, +thus making it obvious which variables are referenced, and what is their +data sharing attribute, thus increasing readability and possibly making errors +easier to spot. + +Example +------- + +.. code-block:: c++ + + // ``for`` directive can not have ``default`` clause, no diagnostics. + void n0(const int a) { + #pragma omp for + for (int b = 0; b < a; b++) + ; + } + + // ``parallel`` directive. + + // ``parallel`` directive can have ``default`` clause, but said clause is not + // specified, diagnosed. + void p0_0() { + #pragma omp parallel + ; + // WARNING: OpenMP directive ``parallel`` does not specify ``default`` + // clause. Consider specifying ``default(none)`` clause. + } + + // ``parallel`` directive can have ``default`` clause, and said clause is + // specified, with ``none`` kind, all good. + void p0_1() { + #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 p0_2() { + #pragma omp parallel default(shared) + ; + // WARNING: OpenMP directive ``parallel`` specifies ``default(shared)`` + // clause.Consider using ``default(none)`` clause instead. + } + + // ``task`` directive. + + // ``task`` directive can have ``default`` clause, but said clause is not + // specified, diagnosed. + void p1_0() { + #pragma omp task + ; + // WARNING: OpenMP directive ``task`` does not specify ``default`` clause. + // Consider specifying ``default(none)`` clause. + } + + // ``task`` directive can have ``default`` clause, and said clause is + // specified, with ``none`` kind, all good. + void p1_1() { + #pragma omp task default(none) + ; + } + + // ``task`` directive can have ``default`` clause, and said clause is + // specified, but with ``shared`` kind, which is not ``none``, diagnose. + void p1_2() { + #pragma omp task default(shared) + ; + // WARNING: OpenMP directive ``task`` specifies ``default(shared)`` clause. + // Consider using ``default(none)`` clause instead. + } + + // ``teams`` directive. (has to be inside of ``target`` directive) + + // ``teams`` directive can have ``default`` clause, but said clause is not + // specified, diagnosed. + void p2_0() { + #pragma omp target + #pragma omp teams + ; + // WARNING: OpenMP directive ``teams`` does not specify ``default`` clause. + // Consider specifying ``default(none)`` clause. + } + + // ``teams`` directive can have ``default`` clause, and said clause is + // specified, with ``none`` kind, all good. + void p2_1() { + #pragma omp target + #pragma omp teams default(none) + ; + } + + // ``teams`` directive can have ``default`` clause, and said clause is + // specified, but with ``shared`` kind, which is not ``none``, diagnose. + void p2_2() { + #pragma omp target + #pragma omp teams default(shared) + ; + // WARNING: OpenMP directive ``teams`` specifies ``default(shared)`` clause. + // Consider using ``default(none)`` clause instead. + } + + // ``taskloop`` directive. + + // ``taskloop`` directive can have ``default`` clause, but said clause is not + // specified, diagnosed. + void p3_0(const int a) { + #pragma omp taskloop + for (int b = 0; b < a; b++) + ; + // WARNING: OpenMP directive ``taskloop`` does not specify ``default`` + // clause. Consider specifying ``default(none)`` clause. + } + + // ``taskloop`` directive can have ``default`` clause, and said clause is + // specified, with ``none`` kind, all good. + void p3_1(const int a) { + #pragma omp taskloop default(none) shared(a) + for (int b = 0; b < a; b++) + ; + } + + // ``taskloop`` directive can have ``default`` clause, and said clause is + // specified, but with ``shared`` kind, which is not ``none``, diagnose. + void p3_2(const int a) { + #pragma omp taskloop default(shared) + for (int b = 0; b < a; b++) + ; + // WARNING: OpenMP directive ``taskloop`` specifies ``default(shared)`` + // clause. Consider using ``default(none)`` clause instead. + } + + // Combined directives are also properly handled: + + // ``parallel`` directive can have ``default`` clause, but said clause is not + // specified, diagnosed. + void p4_0(const int a) { + #pragma omp parallel for + for (int b = 0; b < a; b++) + ; + // WARNING: OpenMP directive ``parallel for`` does not specify ``default`` + // clause. Consider specifying ``default(none)`` clause. + } + + // ``parallel`` directive can have ``default`` clause, and said clause is + // specified, with ``none`` kind, all good. + void p4_1(const int a) { + #pragma omp parallel for default(none) shared(a) + 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 p4_2(const int a) { + #pragma omp parallel for default(shared) + for (int b = 0; b < a; b++) + ; + // WARNING: OpenMP directive ``parallel for`` specifies ``default(shared)`` + // clause. Consider using ``default(none)`` clause instead. + } Index: test/clang-tidy/openmp-use-default-none.cpp =================================================================== --- /dev/null +++ test/clang-tidy/openmp-use-default-none.cpp @@ -0,0 +1,160 @@ +// RUN: %check_clang_tidy %s openmp-use-default-none %t -- -- -x c++ -fopenmp=libomp -fopenmp-version=40 +// RUN: %check_clang_tidy %s openmp-use-default-none %t -- -- -x c -fopenmp=libomp -fopenmp-version=40 + +//----------------------------------------------------------------------------// +// Null cases. +//----------------------------------------------------------------------------// + +// 'for' directive can not have 'default' clause, no diagnostics. +void n0(const int a) { +#pragma omp for + for (int b = 0; b < a; b++) + ; +} + +//----------------------------------------------------------------------------// +// Single-directive positive cases. +//----------------------------------------------------------------------------// + +// 'parallel' directive. + +// 'parallel' directive can have 'default' clause, but said clause is not +// specified, diagnosed. +void p0_0() { +#pragma omp parallel + ; + // CHECK-NOTES: :[[@LINE-2]]:9: warning: OpenMP directive 'parallel' does not specify 'default' clause. Consider specifying 'default(none)' clause. +} + +// 'parallel' directive can have 'default' clause, and said clause is specified, +// with 'none' kind, all good. +void p0_1() { +#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 p0_2() { +#pragma omp parallel default(shared) + ; + // CHECK-NOTES: :[[@LINE-2]]:9: warning: OpenMP directive 'parallel' specifies 'default(shared)' clause. Consider using 'default(none)' clause instead. + // CHECK-NOTES: :[[@LINE-3]]:22: note: Existing 'default' clause is specified here. +} + +// 'task' directive. + +// 'task' directive can have 'default' clause, but said clause is not +// specified, diagnosed. +void p1_0() { +#pragma omp task + ; + // CHECK-NOTES: :[[@LINE-2]]:9: warning: OpenMP directive 'task' does not specify 'default' clause. Consider specifying 'default(none)' clause. +} + +// 'task' directive can have 'default' clause, and said clause is specified, +// with 'none' kind, all good. +void p1_1() { +#pragma omp task default(none) + ; +} + +// 'task' directive can have 'default' clause, and said clause is specified, +// but with 'shared' kind, which is not 'none', diagnose. +void p1_2() { +#pragma omp task default(shared) + ; + // CHECK-NOTES: :[[@LINE-2]]:9: warning: OpenMP directive 'task' specifies 'default(shared)' clause. Consider using 'default(none)' clause instead. + // CHECK-NOTES: :[[@LINE-3]]:18: note: Existing 'default' clause is specified here. +} + +// 'teams' directive. (has to be inside of 'target' directive) + +// 'teams' directive can have 'default' clause, but said clause is not +// specified, diagnosed. +void p2_0() { +#pragma omp target +#pragma omp teams + ; + // CHECK-NOTES: :[[@LINE-2]]:9: warning: OpenMP directive 'teams' does not specify 'default' clause. Consider specifying 'default(none)' clause. +} + +// 'teams' directive can have 'default' clause, and said clause is specified, +// with 'none' kind, all good. +void p2_1() { +#pragma omp target +#pragma omp teams default(none) + ; +} + +// 'teams' directive can have 'default' clause, and said clause is specified, +// but with 'shared' kind, which is not 'none', diagnose. +void p2_2() { +#pragma omp target +#pragma omp teams default(shared) + ; + // CHECK-NOTES: :[[@LINE-2]]:9: warning: OpenMP directive 'teams' specifies 'default(shared)' clause. Consider using 'default(none)' clause instead. + // CHECK-NOTES: :[[@LINE-3]]:19: note: Existing 'default' clause is specified here. +} + +// 'taskloop' directive. + +// 'taskloop' directive can have 'default' clause, but said clause is not +// specified, diagnosed. +void p3_0(const int a) { +#pragma omp taskloop + for (int b = 0; b < a; b++) + ; + // CHECK-NOTES: :[[@LINE-3]]:9: warning: OpenMP directive 'taskloop' does not specify 'default' clause. Consider specifying 'default(none)' clause. +} + +// 'taskloop' directive can have 'default' clause, and said clause is specified, +// with 'none' kind, all good. +void p3_1(const int a) { +#pragma omp taskloop default(none) shared(a) + for (int b = 0; b < a; b++) + ; +} + +// 'taskloop' directive can have 'default' clause, and said clause is specified, +// but with 'shared' kind, which is not 'none', diagnose. +void p3_2(const int a) { +#pragma omp taskloop default(shared) + for (int b = 0; b < a; b++) + ; + // CHECK-NOTES: :[[@LINE-3]]:9: warning: OpenMP directive 'taskloop' specifies 'default(shared)' clause. Consider using 'default(none)' clause instead. + // CHECK-NOTES: :[[@LINE-4]]:22: note: Existing 'default' clause is specified here. +} + +//----------------------------------------------------------------------------// +// Combined directives. +// Let's not test every single possible permutation/combination of directives, +// but just *one* combined directive. The rest will be the same. +//----------------------------------------------------------------------------// + +// 'parallel' directive can have 'default' clause, but said clause is not +// specified, diagnosed. +void p4_0(const int a) { +#pragma omp parallel for + for (int b = 0; b < a; b++) + ; + // CHECK-NOTES: :[[@LINE-3]]:9: warning: OpenMP directive 'parallel for' does not specify 'default' clause. Consider specifying 'default(none)' clause. +} + +// 'parallel' directive can have 'default' clause, and said clause is specified, +// with 'none' kind, all good. +void p4_1(const int a) { +#pragma omp parallel for default(none) shared(a) + 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 p4_2(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' specifies 'default(shared)' clause. Consider using 'default(none)' clause instead. + // CHECK-NOTES: :[[@LINE-4]]:26: note: Existing 'default' clause is specified here. +}