[OpenMP] Fix handling of clause on wrong directive, by Joel. E. Denny

Summary:
First, this patch fixes an assert failure when, for example, "omp for"
has num_teams.

Second, this patch prevents duplicate diagnostics when, for example,
"omp for" has uniform.

This patch makes the general assumption (even where it doesn't
necessarily fix an existing bug) that it is worthless to perform sema
for a clause that appears on a directive on which OpenMP does not
permit that clause.  However, due to this assumption, this patch
suppresses some diagnostics that were expected in the test suite.  I
assert that those diagnostics were likely just distracting to the
user.

Reviewers: ABataev

Reviewed By: ABataev

Subscribers: cfe-commits

Differential Revision: https://reviews.llvm.org/D41841

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@322107 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/Parse/Parser.h b/include/clang/Parse/Parser.h
index 08365cd38051cffb76bb32008619762bf4fa9803..6b722929f60c50a47be241ff5214d2ebdc9eeed3 100644 (file)
--- a/include/clang/Parse/Parser.h
+++ b/include/clang/Parse/Parser.h
@@ -2675,30 +2675,42 @@ private:
   /// \brief Parses clause with a single expression of a kind \a Kind.
   ///
   /// \param Kind Kind of current clause.
+  /// \param ParseOnly true to skip the clause's semantic actions and return
+  /// nullptr.
   ///
-  OMPClause *ParseOpenMPSingleExprClause(OpenMPClauseKind Kind);
+  OMPClause *ParseOpenMPSingleExprClause(OpenMPClauseKind Kind,
+                                         bool ParseOnly);
   /// \brief Parses simple clause of a kind \a Kind.
   ///
   /// \param Kind Kind of current clause.
+  /// \param ParseOnly true to skip the clause's semantic actions and return
+  /// nullptr.
   ///
-  OMPClause *ParseOpenMPSimpleClause(OpenMPClauseKind Kind);
+  OMPClause *ParseOpenMPSimpleClause(OpenMPClauseKind Kind, bool ParseOnly);
   /// \brief Parses clause with a single expression and an additional argument
   /// of a kind \a Kind.
   ///
   /// \param Kind Kind of current clause.
+  /// \param ParseOnly true to skip the clause's semantic actions and return
+  /// nullptr.
   ///
-  OMPClause *ParseOpenMPSingleExprWithArgClause(OpenMPClauseKind Kind);
+  OMPClause *ParseOpenMPSingleExprWithArgClause(OpenMPClauseKind Kind,
+                                                bool ParseOnly);
   /// \brief Parses clause without any additional arguments.
   ///
   /// \param Kind Kind of current clause.
+  /// \param ParseOnly true to skip the clause's semantic actions and return
+  /// nullptr.
   ///
-  OMPClause *ParseOpenMPClause(OpenMPClauseKind Kind);
+  OMPClause *ParseOpenMPClause(OpenMPClauseKind Kind, bool ParseOnly = false);
   /// \brief Parses clause with the list of variables of a kind \a Kind.
   ///
   /// \param Kind Kind of current clause.
+  /// \param ParseOnly true to skip the clause's semantic actions and return
+  /// nullptr.
   ///
   OMPClause *ParseOpenMPVarListClause(OpenMPDirectiveKind DKind,
-                                      OpenMPClauseKind Kind);
+                                      OpenMPClauseKind Kind, bool ParseOnly);
 
 public:
   /// Parses simple expression in parens for single-expression clauses of OpenMP

diff --git a/lib/Parse/ParseOpenMP.cpp b/lib/Parse/ParseOpenMP.cpp
index 628c1f134ac5d183adfed9abe3ec1fd1ab6f1678..f3edf7ca51f40b2751326e4cd800b0ea86ce136d 100644 (file)
--- a/lib/Parse/ParseOpenMP.cpp
+++ b/lib/Parse/ParseOpenMP.cpp
@@ -1205,11 +1205,13 @@ OMPClause *Parser::ParseOpenMPClause(OpenMPDirectiveKind DKind,
                                      OpenMPClauseKind CKind, bool FirstClause) {
   OMPClause *Clause = nullptr;
   bool ErrorFound = false;
+  bool WrongDirective = false;
   // Check if clause is allowed for the given directive.
   if (CKind != OMPC_unknown && !isAllowedClauseForDirective(DKind, CKind)) {
     Diag(Tok, diag::err_omp_unexpected_clause) << getOpenMPClauseName(CKind)
                                                << getOpenMPDirectiveName(DKind);
     ErrorFound = true;
+    WrongDirective = true;
   }
 
   switch (CKind) {
@@ -1253,9 +1255,9 @@ OMPClause *Parser::ParseOpenMPClause(OpenMPDirectiveKind DKind,
     }
 
     if (CKind == OMPC_ordered && PP.LookAhead(/*N=*/0).isNot(tok::l_paren))
-      Clause = ParseOpenMPClause(CKind);
+      Clause = ParseOpenMPClause(CKind, WrongDirective);
     else
-      Clause = ParseOpenMPSingleExprClause(CKind);
+      Clause = ParseOpenMPSingleExprClause(CKind, WrongDirective);
     break;
   case OMPC_default:
   case OMPC_proc_bind:
@@ -1270,7 +1272,7 @@ OMPClause *Parser::ParseOpenMPClause(OpenMPDirectiveKind DKind,
       ErrorFound = true;
     }
 
-    Clause = ParseOpenMPSimpleClause(CKind);
+    Clause = ParseOpenMPSimpleClause(CKind, WrongDirective);
     break;
   case OMPC_schedule:
   case OMPC_dist_schedule:
@@ -1287,7 +1289,7 @@ OMPClause *Parser::ParseOpenMPClause(OpenMPDirectiveKind DKind,
     LLVM_FALLTHROUGH;
 
   case OMPC_if:
-    Clause = ParseOpenMPSingleExprWithArgClause(CKind);
+    Clause = ParseOpenMPSingleExprWithArgClause(CKind, WrongDirective);
     break;
   case OMPC_nowait:
   case OMPC_untied:
@@ -1310,7 +1312,7 @@ OMPClause *Parser::ParseOpenMPClause(OpenMPDirectiveKind DKind,
       ErrorFound = true;
     }
 
-    Clause = ParseOpenMPClause(CKind);
+    Clause = ParseOpenMPClause(CKind, WrongDirective);
     break;
   case OMPC_private:
   case OMPC_firstprivate:
@@ -1330,7 +1332,7 @@ OMPClause *Parser::ParseOpenMPClause(OpenMPDirectiveKind DKind,
   case OMPC_from:
   case OMPC_use_device_ptr:
   case OMPC_is_device_ptr:
-    Clause = ParseOpenMPVarListClause(DKind, CKind);
+    Clause = ParseOpenMPVarListClause(DKind, CKind, WrongDirective);
     break;
   case OMPC_unknown:
     Diag(Tok, diag::warn_omp_extra_tokens_at_eol)
@@ -1339,8 +1341,9 @@ OMPClause *Parser::ParseOpenMPClause(OpenMPDirectiveKind DKind,
     break;
   case OMPC_threadprivate:
   case OMPC_uniform:
-    Diag(Tok, diag::err_omp_unexpected_clause) << getOpenMPClauseName(CKind)
-                                               << getOpenMPDirectiveName(DKind);
+    if (!WrongDirective)
+      Diag(Tok, diag::err_omp_unexpected_clause)
+          << getOpenMPClauseName(CKind) << getOpenMPDirectiveName(DKind);
     SkipUntil(tok::comma, tok::annot_pragma_openmp_end, StopBeforeMatch);
     break;
   }
@@ -1400,7 +1403,8 @@ ExprResult Parser::ParseOpenMPParensExpr(StringRef ClauseName,
 ///    hint-clause:
 ///      'hint' '(' expression ')'
 ///
-OMPClause *Parser::ParseOpenMPSingleExprClause(OpenMPClauseKind Kind) {
+OMPClause *Parser::ParseOpenMPSingleExprClause(OpenMPClauseKind Kind,
+                                               bool ParseOnly) {
   SourceLocation Loc = ConsumeToken();
   SourceLocation LLoc = Tok.getLocation();
   SourceLocation RLoc;
@@ -1410,6 +1414,8 @@ OMPClause *Parser::ParseOpenMPSingleExprClause(OpenMPClauseKind Kind) {
   if (Val.isInvalid())
     return nullptr;
 
+  if (ParseOnly)
+    return nullptr;
   return Actions.ActOnOpenMPSingleExprClause(Kind, Val.get(), Loc, LLoc, RLoc);
 }
 
@@ -1421,7 +1427,8 @@ OMPClause *Parser::ParseOpenMPSingleExprClause(OpenMPClauseKind Kind) {
 ///    proc_bind-clause:
 ///         'proc_bind' '(' 'master' | 'close' | 'spread' ')
 ///
-OMPClause *Parser::ParseOpenMPSimpleClause(OpenMPClauseKind Kind) {
+OMPClause *Parser::ParseOpenMPSimpleClause(OpenMPClauseKind Kind,
+                                           bool ParseOnly) {
   SourceLocation Loc = Tok.getLocation();
   SourceLocation LOpen = ConsumeToken();
   // Parse '('.
@@ -1440,6 +1447,8 @@ OMPClause *Parser::ParseOpenMPSimpleClause(OpenMPClauseKind Kind) {
   // Parse ')'.
   T.consumeClose();
 
+  if (ParseOnly)
+    return nullptr;
   return Actions.ActOnOpenMPSimpleClause(Kind, Type, TypeLoc, LOpen, Loc,
                                          Tok.getLocation());
 }
@@ -1470,10 +1479,12 @@ OMPClause *Parser::ParseOpenMPSimpleClause(OpenMPClauseKind Kind) {
 ///    nogroup-clause:
 ///         'nogroup'
 ///
-OMPClause *Parser::ParseOpenMPClause(OpenMPClauseKind Kind) {
+OMPClause *Parser::ParseOpenMPClause(OpenMPClauseKind Kind, bool ParseOnly) {
   SourceLocation Loc = Tok.getLocation();
   ConsumeAnyToken();
 
+  if (ParseOnly)
+    return nullptr;
   return Actions.ActOnOpenMPClause(Kind, Loc, Tok.getLocation());
 }
 
@@ -1491,7 +1502,8 @@ OMPClause *Parser::ParseOpenMPClause(OpenMPClauseKind Kind) {
 ///    defaultmap:
 ///      'defaultmap' '(' modifier ':' kind ')'
 ///
-OMPClause *Parser::ParseOpenMPSingleExprWithArgClause(OpenMPClauseKind Kind) {
+OMPClause *Parser::ParseOpenMPSingleExprWithArgClause(OpenMPClauseKind Kind,
+                                                      bool ParseOnly) {
   SourceLocation Loc = ConsumeToken();
   SourceLocation DelimLoc;
   // Parse '('.
@@ -1613,6 +1625,8 @@ OMPClause *Parser::ParseOpenMPSingleExprWithArgClause(OpenMPClauseKind Kind) {
   if (NeedAnExpression && Val.isInvalid())
     return nullptr;
 
+  if (ParseOnly)
+    return nullptr;
   return Actions.ActOnOpenMPSingleExprWithArgClause(
       Kind, Arg, Val.get(), Loc, T.getOpenLocation(), KLoc, DelimLoc,
       T.getCloseLocation());
@@ -1940,7 +1954,8 @@ bool Parser::ParseOpenMPVarList(OpenMPDirectiveKind DKind,
 ///  modifier(list)
 /// where modifier is 'val' (C) or 'ref', 'val' or 'uval'(C++).
 OMPClause *Parser::ParseOpenMPVarListClause(OpenMPDirectiveKind DKind,
-                                            OpenMPClauseKind Kind) {
+                                            OpenMPClauseKind Kind,
+                                            bool ParseOnly) {
   SourceLocation Loc = Tok.getLocation();
   SourceLocation LOpen = ConsumeToken();
   SmallVector<Expr *, 4> Vars;
@@ -1949,6 +1964,8 @@ OMPClause *Parser::ParseOpenMPVarListClause(OpenMPDirectiveKind DKind,
   if (ParseOpenMPVarList(DKind, Kind, Vars, Data))
     return nullptr;
 
+  if (ParseOnly)
+    return nullptr;
   return Actions.ActOnOpenMPVarListClause(
       Kind, Vars, Data.TailExpr, Loc, LOpen, Data.ColonLoc, Tok.getLocation(),
       Data.ReductionIdScopeSpec, Data.ReductionId, Data.DepKind, Data.LinKind,

diff --git a/test/OpenMP/distribute_simd_loop_messages.cpp b/test/OpenMP/distribute_simd_loop_messages.cpp
index 1ae28a97872efe4af5910490c9563c13c82db502..1977ca789dcf91da067c48c618da011a0dd56fdc 100644 (file)
--- a/test/OpenMP/distribute_simd_loop_messages.cpp
+++ b/test/OpenMP/distribute_simd_loop_messages.cpp
@@ -324,9 +324,7 @@ int test_iteration_spaces() {
 
   #pragma omp target
   #pragma omp teams
-  // expected-error@+3 {{unexpected OpenMP clause 'shared' in directive '#pragma omp distribute simd'}}
-  // expected-note@+2  {{defined as shared}}
-  // expected-error@+2 {{loop iteration variable in the associated loop of 'omp distribute simd' directive may not be shared, predetermined as linear}}
+  // expected-error@+1 {{unexpected OpenMP clause 'shared' in directive '#pragma omp distribute simd'}}
   #pragma omp distribute simd shared(ii)
   for (ii = 0; ii < 10; ii++)
     c[ii] = a[ii];

diff --git a/test/OpenMP/for_misc_messages.c b/test/OpenMP/for_misc_messages.c
index ffcc639e8ebe0f4799a57772be8d92c8925e2758..901d66dd688cf26af8049454f8599c3d9011e467 100644 (file)
--- a/test/OpenMP/for_misc_messages.c
+++ b/test/OpenMP/for_misc_messages.c
@@ -54,6 +54,20 @@ void test_invalid_clause() {
 #pragma omp for foo bar
   for (i = 0; i < 16; ++i)
     ;
+// At one time, this failed an assert.
+// expected-error@+1 {{unexpected OpenMP clause 'num_teams' in directive '#pragma omp for'}}
+#pragma omp for num_teams(3)
+  for (i = 0; i < 16; ++i)
+    ;
+// At one time, this error was reported twice.
+// expected-error@+1 {{unexpected OpenMP clause 'uniform' in directive '#pragma omp for'}}
+#pragma omp for uniform
+  for (i = 0; i < 16; ++i)
+    ;
+// expected-error@+1 {{unexpected OpenMP clause 'if' in directive '#pragma omp for'}}
+#pragma omp for if(0)
+  for (i = 0; i < 16; ++i)
+    ;
 }
 
 void test_non_identifiers() {

diff --git a/test/OpenMP/parallel_messages.cpp b/test/OpenMP/parallel_messages.cpp
index 959ce11be8833120ad6326316b1006f76ab02951..8b0c0353c20d6bd607e8a7199421ba569efbafcb 100644 (file)
--- a/test/OpenMP/parallel_messages.cpp
+++ b/test/OpenMP/parallel_messages.cpp
@@ -31,6 +31,8 @@ int main(int argc, char **argv) {
   foo();
   L1:
     foo();
+  #pragma omp parallel ordered // expected-error {{unexpected OpenMP clause 'ordered' in directive '#pragma omp parallel'}}
+    ;
   #pragma omp parallel
   ;
   #pragma omp parallel

diff --git a/test/OpenMP/simd_loop_messages.cpp b/test/OpenMP/simd_loop_messages.cpp
index 5a2df073abbf99af89fa9cfb93f0b31d02ffba11..b9d146c2150a6849357ff1f4154e6045b3750ff3 100644 (file)
--- a/test/OpenMP/simd_loop_messages.cpp
+++ b/test/OpenMP/simd_loop_messages.cpp
@@ -236,9 +236,7 @@ int test_iteration_spaces() {
   for (ii = 0; ii < 10; ii++)
     c[ii] = a[ii];
 
-  // expected-error@+3 {{unexpected OpenMP clause 'shared' in directive '#pragma omp simd'}}
-  // expected-note@+2  {{defined as shared}}
-  // expected-error@+2 {{loop iteration variable in the associated loop of 'omp simd' directive may not be shared, predetermined as linear}}
+  // expected-error@+1 {{unexpected OpenMP clause 'shared' in directive '#pragma omp simd'}}
   #pragma omp simd shared(ii)
   for (ii = 0; ii < 10; ii++)
     c[ii] = a[ii];

diff --git a/test/OpenMP/target_teams_distribute_simd_messages.cpp b/test/OpenMP/target_teams_distribute_simd_messages.cpp
index adef1feb3b56ead5bf75d5b9b279aac0663d7b22..6ee8073357f4cda94d625493f336954fd1b912de 100644 (file)
--- a/test/OpenMP/target_teams_distribute_simd_messages.cpp
+++ b/test/OpenMP/target_teams_distribute_simd_messages.cpp
@@ -64,7 +64,7 @@ L1:
 // expected-error@+1 {{unexpected OpenMP clause 'default' in directive '#pragma omp target teams distribute simd'}}
 #pragma omp target teams distribute simd default(none)
   for (int i = 0; i < 10; ++i)
-    ++argc; // expected-error {{ariable 'argc' must have explicitly specified data sharing attributes}}
+    ++argc;
 
   goto L2; // expected-error {{use of undeclared label 'L2'}}
 #pragma omp target teams distribute simd

diff --git a/test/OpenMP/taskloop_loop_messages.cpp b/test/OpenMP/taskloop_loop_messages.cpp
index 00c7d716b9662efe9191328b7df2105fd0b3a9b4..4ce83b4ae1aeb9df601f474dd39176229a5a7924 100644 (file)
--- a/test/OpenMP/taskloop_loop_messages.cpp
+++ b/test/OpenMP/taskloop_loop_messages.cpp
@@ -294,10 +294,8 @@ int test_iteration_spaces() {
     c[ii] = a[ii];
 
 #pragma omp parallel
-// expected-error@+2 {{unexpected OpenMP clause 'linear' in directive '#pragma omp taskloop'}}
-// expected-note@+1 {{defined as linear}}
+// expected-error@+1 {{unexpected OpenMP clause 'linear' in directive '#pragma omp taskloop'}}
 #pragma omp taskloop linear(ii)
-// expected-error@+1 {{loop iteration variable in the associated loop of 'omp taskloop' directive may not be linear, predetermined as private}}
   for (ii = 0; ii < 10; ii++)
     c[ii] = a[ii];