[clang-tidy] openmp-use-default-none - a new check

[lebedev.ri]

Summary

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.

[Eugene.Zelenko]

Please mention new module in docs/clang-tidy/index.rst.

docs/ReleaseNotes.rst

86

Please put new module before list of new checks.

93

Please use `` for default(none). Will be good idea to make this statement same as in documentation.

docs/clang-tidy/checks/openmp-use-default-none.rst

20

Unnecessary empty line.

[lebedev.ri]

Fix documentation nits.

[JonasToth]

clang-tidy/openmp/UseDefaultNoneCheck.cpp

52

Why is this required? If it is not allowed, is the shouldn't that be a compilation error already (always assuming openmp is activated).

98

you could provide helper-matchers similiar to isImplicit(), isInline() to allow ompDefaultClause(isDefaultShared()).

132

assert without message, but probably redundant anyway, because the matcher can only fire if directive matched.

136

the message is too long, how about consider specifiying clause 'default(none)' explicitly or so?
Adding that it would be allowed is redundant, as one expects the tool to know that and consider that correctly and not diagnose otherwise.

[JonasToth]

test/clang-tidy/openmp-use-default-none.cpp

27

AFAIK default(private) should exist as well, please add tests for the other kinds, too.

68

why rewrite the default(shared)? I don't exactly understand the reason for not accepting default(shared).

[ABataev]

test/clang-tidy/openmp-use-default-none.cpp

27

No, only default(none|shared) exists

[lebedev.ri]

clang-tidy/openmp/UseDefaultNoneCheck.cpp

52

It's tricky logic.
There are three possible scenarios here:

• default(none) is specified, all good.
• default(shared) is specified, shared!=none, diagnose.
• No default clause specified. Only a single default clause may be specified on a parallel, task, taskloop or teams directive. ([[ https://www.openmp.org/wp-content/uploads/OpenMP-API-Specification-5.0.pdf | 2.19.4.1 default Clause ]], last line of paragraph). Now, there are two possible cases:
  • We are in a directive (e.g. parallel) that is allowed to have the default clause, but does not have it. Naturally, do diagnose this.
  • We are in a directive (e.g. for) that is not allowed to have the default clause. Naturally, don't diagnose this.

So this is correct.

98

Could work, will take a look, thanks for a hint!

132

Hm, i can drop it, but in previous reviews i have seen this assert being requested to be *added*.

136

Good point about "allowed", i'll see how this can be trimmed..

test/clang-tidy/openmp-use-default-none.cpp

27

In Fortran - yes, but not in C / C++:
https://www.openmp.org/wp-content/uploads/OpenMP-API-Specification-5.0.pdf
Page 282 (302 in pdf):

2.19.4.1 default Clause
...
C / C++
The syntax of the default clause is as follows:
default(shared | none)

68

This check enforces default(none). If the default of shared is specified,
that is still not none, therefore it is incorrect and should be diagnosed.

Why not shared but none? I tried to cover that in the doc,
basically it helps prevent issues by forcing one to be explicit about the "visibilities"
of the variables.

I suppose it the default can be configurable,
i'm just not aware of any reason why one would specify shared.

[lebedev.ri]

@JonasToth thanks for taking a look BTW!

[JonasToth]

docs/clang-tidy/checks/openmp-use-default-none.rst

13

If I understand correctly the issue is more about implicitly shared variables that lead to data-races and bad access patterns, is that correct?
If so it might clarify the reason for this check, if added directly in the first motivational sentence.

test/clang-tidy/openmp-use-default-none.cpp

68

I have the honor to work on a originally fortran77 scientific code (which is getting modernized step by step) but there it is very common to have 100 variables with maximum length of 4 characters for the variable names.
One would expect the same quality for similar C-Code when OMP beeing used, so there might be the practical reason that 20 lines of declarations what variable is what are unreadable (because the rest is a big desaster already). But in that case one will not use this check..

[JonasToth]

clang-tidy/openmp/UseDefaultNoneCheck.cpp

114

Is that the case? Will the pragmas be ignored if non-omp?

You could reorder that sentence Don't register the check if OpenMP is not enabled as the directives are not considered in the AST. or so.

132

You can leave it, but please add an message to the assertion.

clang-tidy/openmp/UseDefaultNoneCheck.h

19

contain _a_ `default` clause

20

double back-tick in dont, please use the normal straight tick instead

21

to use _the_

docs/clang-tidy/checks/openmp-use-default-none.rst

7

not sure, but i think all the commas in this sentence can be dropped. The comma in front of or for sure.

12

forces _developers_

13

is it scoping or rather sharing? The scope is C++-terrain or at least used in C++-Speak. Maybe there is a unambiguous word instead?

29

Very long line. You can move the warning to the next line and wrap it to match the 80 columns limit.

test/clang-tidy/openmp-use-default-none.cpp

54

Please add basic test-cases for the other constructs that are allowed to have the default specified.

[ABataev]

clang-tidy/openmp/UseDefaultNoneCheck.cpp

114

If OpenMP is disabled, the pragmas are just completely ignored.

[lebedev.ri]

docs/clang-tidy/checks/openmp-use-default-none.rst

13

I'm not quite sure how to formulate the reasoning to be honest.
Let me try to show some reasonably-complete example:
https://godbolt.org/z/mMQhbr

We have 4 compilers: clang trunk + openmp 3.1, clang trunk + openmp 4, gcc 8, gcc trunk

We have 5 code samples, all operating with a const variable: (the same code, just different omp clauses)

• If no default clause is specified, every compiler is fine with the code.
• If default(shared) clause is specified, every compiler is still fine with the code. On this example, no clause and default(shared) clause are equivalent. I can't/won't claim whether or not that is always the case, as i'm mostly arguing re default(none).
• If default(none) shared(num) is specified, all is also fine. That is equivalent to just the default(none) for OpenMP 3.1
• If default(none) firstprivate(num) is specified, all still fine fine.
• If only `default(none) is specified, things start to get wonky. The general idea is that before OpenMP 4.0 such const variables were auto-determined as shared, and now they won't. So one will need to add either shared(num) or firstprivate(num). Except the older gcc will diagnose shared(num) :)

Roughly the same is true when the variable is not const: https://godbolt.org/z/wuouI_

Thus, default(none) forced one to be explicit about what shall be done with the variable,
should it be shared, or firstprivate.

^ Not whether this rambling makes sense?

test/clang-tidy/openmp-use-default-none.cpp

68

Oh yeah, i'm quite sure that is common for non-fortran code too.

[JonasToth]

docs/clang-tidy/checks/openmp-use-default-none.rst

13

Yes, so its about being explicit if state is shared or not. Given that its hard to reason about parallel programs being explicit helps reading the code. I think that could be condensed in a short motivation section.

[lebedev.ri]

docs/clang-tidy/checks/openmp-use-default-none.rst

13

Yes, so its about being explicit if state is shared or not. Given that its hard to reason about parallel programs being explicit helps reading the code. I think that could be condensed in a short motivation section.

Yep. Will try to reword.

is it scoping or rather sharing? The scope is C++-terrain or at least used in C++-Speak. Maybe there is a unambiguous word instead?

E.g. clang messages say variable 'num' must have explicitly specified data sharing attributes
So "data sharing" i suppose.

test/clang-tidy/openmp-use-default-none.cpp

54

For the base directives (so 3x4 new tests) i assume,
not all the possible combinations of directives that are allowed to contain default (a *LOT* more tests)?

[lebedev.ri]

Addressed review notes;
found a way to use the OMPClause matcher in isAllowedToContainClause(),
as opposed to passing the OpenMPClauseKind.

clang-tidy/openmp/UseDefaultNoneCheck.cpp

50–51

Solved, this is so ugly xD

98

Not quite sure re Kind suffix, but i guess this is better.

114

Reworded a little, hopefully better.

clang-tidy/openmp/UseDefaultNoneCheck.h

20

Hah, that is mass-replace for you.

docs/clang-tidy/checks/openmp-use-default-none.rst

13

Reworded, hopefully better?

[lebedev.ri]

Adjust a few comments.

[Eugene.Zelenko]

New module is still not mentioned in docs/clang-tidy/index.rst.

[lebedev.ri]

Mention new module in docs/clang-tidy/index.rst.

[JonasToth]

LGTM with the few language nits.
The new matchers look better and are more matcher style, gj :)

clang-tidy/openmp/UseDefaultNoneCheck.cpp

127

typo, entries

docs/clang-tidy/checks/openmp-use-default-none.rst

11

Using the

15

making errors easier to spot. the more is not necessary.

[lebedev.ri]

In D57113#1372891, @JonasToth wrote:

LGTM with the few language nits.
The new matchers look better and are more matcher style, gj :)

Thank you for the review!
I'm not quite sure who is qualified to review the base
[[ https://reviews.llvm.org/D57112 | D57112 ASTTypeTraits ]] patch though, @klimek or @sbenza it looks like?

[lebedev.ri]

NFC.

• Rebased
• Switched to OpenMPClauseKind ASTNodeKind::getOMPClauseKind(). This is better anyway, since it avoids string comparisons, and is a simple switch over an enum.

[lebedev.ri]

NFC, rebased, split the new check from the new module into separate differentials.

[lebedev.ri]

Rebased, NFC.
Moved matchers into D59453+D57112.

[lebedev.ri]

Rebased, NFC.

[lebedev.ri]

Address some nits.

[aaron.ballman]

clang-tidy/openmp/UseDefaultNoneCheck.cpp

48–49

Make the diagnostic ungrammatical.

53

Same with notes.

59–60

Here too.

docs/clang-tidy/checks/openmp-use-default-none.rst

20

This is a *lot* of example text -- are you sure all of it adds value, or can some of it be removed?

[lebedev.ri]

Make diags less grammatically correct, reduce docs.

docs/clang-tidy/checks/openmp-use-default-none.rst

20

Right, let's just leave one example :)

[aaron.ballman]

LGTM aside from a minor diagnostic wording nit.

clang-tidy/openmp/UseDefaultNoneCheck.cpp

53

drop the "is" to make it match how we usually phrase things.

[lebedev.ri]

In D57113#1439890, @aaron.ballman wrote:

LGTM aside from a minor diagnostic wording nit.

Thank you for the review!

clang-tidy/openmp/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS support)
 
 add_clang_library(clangTidyOpenMPModule
   OpenMPTidyModule.cpp
+  UseDefaultNoneCheck.cpp
 
   LINK_LIBS
   clangAST
   clangASTMatchers
   clangBasic
   clangTidy
   )

clang-tidy/openmp/OpenMPTidyModule.cpp

 //===--- 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<UseDefaultNoneCheck>(
+        "openmp-use-default-none");
   }
 };
 
 // Register the OpenMPTidyModule using this statically initialized variable.
 static ClangTidyModuleRegistry::Add<OpenMPModule>
     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

clang-tidy/openmp/UseDefaultNoneCheck.h

+//===--- 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

clang-tidy/openmp/UseDefaultNoneCheck.cpp

+//===--- 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

docs/ReleaseNotes.rst

- New :doc:`abseil-duration-addition
   domain.
 
 - New :doc:`abseil-duration-conversion-cast
   <clang-tidy/checks/abseil-duration-conversion-cast>` check.
 
   Checks for casts of ``absl::Duration`` conversion functions, and recommends
   the right conversion function instead.
 
 - New :doc:`abseil-duration-unnecessary-conversion
   <clang-tidy/checks/abseil-duration-unnecessary-conversion>` check.
 
   Finds and fixes cases where ``absl::Duration`` values are being converted to
   numeric types and back again.
 
 - New :doc:`abseil-time-comparison
   <clang-tidy/checks/abseil-time-comparison>` check.
 
   Prefer comparisons in the ``absl::Time`` domain instead of the integer
   domain.
 
 - New :doc:`abseil-time-subtraction
   <clang-tidy/checks/abseil-time-subtraction>` check.
 
   Finds and fixes ``absl::Time`` subtraction expressions to do subtraction

 - The `Acronyms` and `IncludeDefaultAcronyms` options for the
   :doc:`objc-property-declaration <clang-tidy/checks/objc-property-declaration>`
   check have been removed.
 
 - The :doc:`modernize-use-override
   <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
 -----------------------------
 
 The improvements are...
 
 Improvements to modularize
 --------------------------
 
 The improvements are...
 
 Improvements to pp-trace
 ------------------------
 
 - Added a new option `-callbacks` to filter preprocessor callbacks. It replaces
   the `-ignore` option.

docs/clang-tidy/checks/list.rst

.. toctree::
    modernize-use-uncaught-exceptions
    modernize-use-using
    mpi-buffer-deref
    mpi-type-mismatch
    objc-avoid-nserror-init
    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
    performance-inefficient-algorithm
    performance-inefficient-string-concatenation
    performance-inefficient-vector-operation
    performance-move-const-arg
    performance-move-constructor-init

docs/clang-tidy/checks/openmp-use-default-none.rst

+.. 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.
+  }

test/clang-tidy/openmp-use-default-none.cpp

+// 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
+}