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

Alexey Bataev via cfe-commits cfe-commits at lists.llvm.org
Tue Jan 9 11:21:04 PST 2018


Author: abataev
Date: Tue Jan  9 11:21:04 2018
New Revision: 322107

URL: http://llvm.org/viewvc/llvm-project?rev=322107&view=rev
Log:
[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

Modified:
    cfe/trunk/include/clang/Parse/Parser.h
    cfe/trunk/lib/Parse/ParseOpenMP.cpp
    cfe/trunk/test/OpenMP/distribute_simd_loop_messages.cpp
    cfe/trunk/test/OpenMP/for_misc_messages.c
    cfe/trunk/test/OpenMP/parallel_messages.cpp
    cfe/trunk/test/OpenMP/simd_loop_messages.cpp
    cfe/trunk/test/OpenMP/target_teams_distribute_simd_messages.cpp
    cfe/trunk/test/OpenMP/taskloop_loop_messages.cpp

Modified: cfe/trunk/include/clang/Parse/Parser.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Parse/Parser.h?rev=322107&r1=322106&r2=322107&view=diff
==============================================================================
--- cfe/trunk/include/clang/Parse/Parser.h (original)
+++ cfe/trunk/include/clang/Parse/Parser.h Tue Jan  9 11:21:04 2018
@@ -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

Modified: cfe/trunk/lib/Parse/ParseOpenMP.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseOpenMP.cpp?rev=322107&r1=322106&r2=322107&view=diff
==============================================================================
--- cfe/trunk/lib/Parse/ParseOpenMP.cpp (original)
+++ cfe/trunk/lib/Parse/ParseOpenMP.cpp Tue Jan  9 11:21:04 2018
@@ -1205,11 +1205,13 @@ OMPClause *Parser::ParseOpenMPClause(Ope
                                      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(Ope
     }
 
     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(Ope
       ErrorFound = true;
     }
 
-    Clause = ParseOpenMPSimpleClause(CKind);
+    Clause = ParseOpenMPSimpleClause(CKind, WrongDirective);
     break;
   case OMPC_schedule:
   case OMPC_dist_schedule:
@@ -1287,7 +1289,7 @@ OMPClause *Parser::ParseOpenMPClause(Ope
     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(Ope
       ErrorFound = true;
     }
 
-    Clause = ParseOpenMPClause(CKind);
+    Clause = ParseOpenMPClause(CKind, WrongDirective);
     break;
   case OMPC_private:
   case OMPC_firstprivate:
@@ -1330,7 +1332,7 @@ OMPClause *Parser::ParseOpenMPClause(Ope
   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(Ope
     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
 ///    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::ParseOpenMPSingleExpr
   if (Val.isInvalid())
     return nullptr;
 
+  if (ParseOnly)
+    return nullptr;
   return Actions.ActOnOpenMPSingleExprClause(Kind, Val.get(), Loc, LLoc, RLoc);
 }
 
@@ -1421,7 +1427,8 @@ OMPClause *Parser::ParseOpenMPSingleExpr
 ///    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::ParseOpenMPSimpleClau
   // Parse ')'.
   T.consumeClose();
 
+  if (ParseOnly)
+    return nullptr;
   return Actions.ActOnOpenMPSimpleClause(Kind, Type, TypeLoc, LOpen, Loc,
                                          Tok.getLocation());
 }
@@ -1470,10 +1479,12 @@ OMPClause *Parser::ParseOpenMPSimpleClau
 ///    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(Ope
 ///    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::ParseOpenMPSingleExpr
   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(OpenMPDi
 ///  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::ParseOpenMPVarListCla
   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,

Modified: cfe/trunk/test/OpenMP/distribute_simd_loop_messages.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/distribute_simd_loop_messages.cpp?rev=322107&r1=322106&r2=322107&view=diff
==============================================================================
--- cfe/trunk/test/OpenMP/distribute_simd_loop_messages.cpp (original)
+++ cfe/trunk/test/OpenMP/distribute_simd_loop_messages.cpp Tue Jan  9 11:21:04 2018
@@ -324,9 +324,7 @@ int test_iteration_spaces() {
 
   #pragma omp target
   #pragma omp teams
-  // expected-error at +3 {{unexpected OpenMP clause 'shared' in directive '#pragma omp distribute simd'}}
-  // expected-note at +2  {{defined as shared}}
-  // expected-error at +2 {{loop iteration variable in the associated loop of 'omp distribute simd' directive may not be shared, predetermined as linear}}
+  // expected-error at +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];

Modified: cfe/trunk/test/OpenMP/for_misc_messages.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/for_misc_messages.c?rev=322107&r1=322106&r2=322107&view=diff
==============================================================================
--- cfe/trunk/test/OpenMP/for_misc_messages.c (original)
+++ cfe/trunk/test/OpenMP/for_misc_messages.c Tue Jan  9 11:21:04 2018
@@ -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 at +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 at +1 {{unexpected OpenMP clause 'uniform' in directive '#pragma omp for'}}
+#pragma omp for uniform
+  for (i = 0; i < 16; ++i)
+    ;
+// expected-error at +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() {

Modified: cfe/trunk/test/OpenMP/parallel_messages.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/parallel_messages.cpp?rev=322107&r1=322106&r2=322107&view=diff
==============================================================================
--- cfe/trunk/test/OpenMP/parallel_messages.cpp (original)
+++ cfe/trunk/test/OpenMP/parallel_messages.cpp Tue Jan  9 11:21:04 2018
@@ -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

Modified: cfe/trunk/test/OpenMP/simd_loop_messages.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/simd_loop_messages.cpp?rev=322107&r1=322106&r2=322107&view=diff
==============================================================================
--- cfe/trunk/test/OpenMP/simd_loop_messages.cpp (original)
+++ cfe/trunk/test/OpenMP/simd_loop_messages.cpp Tue Jan  9 11:21:04 2018
@@ -236,9 +236,7 @@ int test_iteration_spaces() {
   for (ii = 0; ii < 10; ii++)
     c[ii] = a[ii];
 
-  // expected-error at +3 {{unexpected OpenMP clause 'shared' in directive '#pragma omp simd'}}
-  // expected-note at +2  {{defined as shared}}
-  // expected-error at +2 {{loop iteration variable in the associated loop of 'omp simd' directive may not be shared, predetermined as linear}}
+  // expected-error at +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];

Modified: cfe/trunk/test/OpenMP/target_teams_distribute_simd_messages.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/target_teams_distribute_simd_messages.cpp?rev=322107&r1=322106&r2=322107&view=diff
==============================================================================
--- cfe/trunk/test/OpenMP/target_teams_distribute_simd_messages.cpp (original)
+++ cfe/trunk/test/OpenMP/target_teams_distribute_simd_messages.cpp Tue Jan  9 11:21:04 2018
@@ -64,7 +64,7 @@ L1:
 // expected-error at +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

Modified: cfe/trunk/test/OpenMP/taskloop_loop_messages.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/taskloop_loop_messages.cpp?rev=322107&r1=322106&r2=322107&view=diff
==============================================================================
--- cfe/trunk/test/OpenMP/taskloop_loop_messages.cpp (original)
+++ cfe/trunk/test/OpenMP/taskloop_loop_messages.cpp Tue Jan  9 11:21:04 2018
@@ -294,10 +294,8 @@ int test_iteration_spaces() {
     c[ii] = a[ii];
 
 #pragma omp parallel
-// expected-error at +2 {{unexpected OpenMP clause 'linear' in directive '#pragma omp taskloop'}}
-// expected-note at +1 {{defined as linear}}
+// expected-error at +1 {{unexpected OpenMP clause 'linear' in directive '#pragma omp taskloop'}}
 #pragma omp taskloop linear(ii)
-// expected-error at +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];
 




More information about the cfe-commits mailing list