lebedev.ri updated this revision to Diff 190973.
lebedev.ri edited the summary of this revision.
lebedev.ri added a comment.
Rebased, NFC.
Moved matchers into D59453 <https://reviews.llvm.org/D59453>+D57112
<https://reviews.llvm.org/D57112>.
Repository:
rCTE Clang Tools Extra
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D57113/new/
https://reviews.llvm.org/D57113
Files:
clang-tidy/openmp/CMakeLists.txt
clang-tidy/openmp/OpenMPTidyModule.cpp
clang-tidy/openmp/UseDefaultNoneCheck.cpp
clang-tidy/openmp/UseDefaultNoneCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/openmp-use-default-none.rst
test/clang-tidy/openmp-use-default-none.cpp
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 -fopenmp-version=40
+// RUN: %check_clang_tidy %s openmp-use-default-none %t -- -- -x c -fopenmp -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.
+}
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
+more 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: 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/ReleaseNotes.rst
===================================================================
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -127,6 +127,13 @@
<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
-----------------------------
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(isAllowedToContainClause(ompDefaultClause()),
+ anyOf(unless(hasClause(ompDefaultClause())),
+ hasClause(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 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: 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<UseDefaultNoneCheck>(
+ "openmp-use-default-none");
}
};
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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
