diff options
8 files changed, 326 insertions, 0 deletions
diff --git a/clang-tools-extra/clang-tidy/openmp/CMakeLists.txt b/clang-tools-extra/clang-tidy/openmp/CMakeLists.txt index 4bd4dffead8..7522be3a467 100644 --- a/clang-tools-extra/clang-tidy/openmp/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/openmp/CMakeLists.txt @@ -2,6 +2,7 @@ set(LLVM_LINK_COMPONENTS support) add_clang_library(clangTidyOpenMPModule OpenMPTidyModule.cpp + UseDefaultNoneCheck.cpp LINK_LIBS clangAST diff --git a/clang-tools-extra/clang-tidy/openmp/OpenMPTidyModule.cpp b/clang-tools-extra/clang-tidy/openmp/OpenMPTidyModule.cpp index 9dcc25885bd..d200f49f8ce 100644 --- a/clang-tools-extra/clang-tidy/openmp/OpenMPTidyModule.cpp +++ b/clang-tools-extra/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 @@ namespace openmp { class OpenMPModule : public ClangTidyModule { public: void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override { + CheckFactories.registerCheck<UseDefaultNoneCheck>( + "openmp-use-default-none"); } }; diff --git a/clang-tools-extra/clang-tidy/openmp/UseDefaultNoneCheck.cpp b/clang-tools-extra/clang-tidy/openmp/UseDefaultNoneCheck.cpp new file mode 100644 index 00000000000..d25498d5fc5 --- /dev/null +++ b/clang-tools-extra/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<OMPExecutableDirective>("directive"); + assert(Directive != nullptr && "Expected to match some directive."); + + if (const auto *Clause = Result.Nodes.getNodeAs<OMPDefaultClause>("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 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 diff --git a/clang-tools-extra/clang-tidy/openmp/UseDefaultNoneCheck.h b/clang-tools-extra/clang-tidy/openmp/UseDefaultNoneCheck.h new file mode 100644 index 00000000000..c5ff8829cd3 --- /dev/null +++ b/clang-tools-extra/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 diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index d85ba1bd546..fcbf6d9507e 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -130,6 +130,13 @@ Improvements to clang-tidy <clang-tidy/checks/modernize-use-override>` now supports `OverrideSpelling` and `FinalSpelling` options. +- New :doc:`openmp-use-default-none + <clang-tidy/checks/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 ----------------------------- diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index 80aba4a0edf..428c22c7fdd 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -227,6 +227,7 @@ Clang-Tidy Checks 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 diff --git a/clang-tools-extra/docs/clang-tidy/checks/openmp-use-default-none.rst b/clang-tools-extra/docs/clang-tidy/checks/openmp-use-default-none.rst new file mode 100644 index 00000000000..4223a10bd6e --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/openmp-use-default-none.rst @@ -0,0 +1,53 @@ +.. 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. + } diff --git a/clang-tools-extra/test/clang-tidy/openmp-use-default-none.cpp b/clang-tools-extra/test/clang-tidy/openmp-use-default-none.cpp new file mode 100644 index 00000000000..1a374bde94a --- /dev/null +++ b/clang-tools-extra/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 specified, +// with 'none' kind, all good. +void p0_1() { +#pragma omp parallel default(none) + ; +} + +// 'parallel' directive can have 'default' clause, and said clause 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 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 specified, +// with 'none' kind, all good. +void p1_1() { +#pragma omp task default(none) + ; +} + +// 'task' directive can have 'default' clause, and said clause 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 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 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 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 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 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 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 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 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 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 specified here +} |
