[OpenMP 5.2] Deprecate 'destroy' clause without argument for 'depobj' construct

[mdfazlay]

Summary

Deprecate and diagnose uses of the 'destroy' clauses without an argument in 'omp depobj' directives.

Note that an error is given for deprecated features (rather than a warning).

Reference:
(1) OpenMP 5.2 Sepec: Section 3.5, Page 73
(2) OpenMP 5.2 spec: Sec B.2 line 13, Page 626

[ABataev]

clang/include/clang/Basic/DiagnosticParseKinds.td

496

1. Usually we name them err_omp_... Also, move to other err_omp messages.
2. Better to use something like "Expected depobj with argument if destroy clause is specified" or something like this.

clang/lib/Sema/SemaOpenMP.cpp

17666

Can it be used with any other directive except for depobj?

[mdfazlay]

Updated the diff. Please take a look.

clang/include/clang/Basic/DiagnosticParseKinds.td

496

Done :)

clang/lib/Sema/SemaOpenMP.cpp

17666

I tried to make it more general. Please take a look.

[ABataev]

clang/include/clang/Basic/DiagnosticParseKinds.td

1374

We do not use OpenMP versions in messages.

clang/lib/Sema/SemaOpenMP.cpp

17665–17666

You can drop some parens inside condition

[mdfazlay]

@ABataev, I have addressed your last two comments and updated the diff. Please take a look at your convenient time :)

[ABataev]

LG

clang/include/clang/Basic/DiagnosticParseKinds.td

 def err_arc_bridge_retain : Error<
   "unknown cast annotation __bridge_retain; did you mean __bridge_retained?">;
 // To be default mapped to an error later.
 def warn_arc_bridge_cast_nonarc : Warning<
   "'%0' casts have no effect when not using ARC">,
   InGroup<DiagGroup<"arc-bridge-casts-disallowed-in-nonarc">>;
 }
 
 def err_objc_illegal_visibility_spec : Error<
   "illegal visibility specification">;
 def err_objc_illegal_interface_qual : Error<"illegal interface qualifier">;
 def err_objc_expected_equal_for_getter : Error<
   "expected '=' for Objective-C getter">;
 def err_objc_expected_equal_for_setter : Error<
   "expected '=' for Objective-C setter">;
 def err_objc_expected_selector_for_getter_setter : Error<
   "expected selector for Objective-C %select{setter|getter}0">;
def err_omp_unknown_map_type_modifier : Error<
   "incorrect map type modifier, expected one of: 'always', 'close', 'mapper'"
   "%select{|, 'present'|, 'present', 'iterator'}0%select{|, 'ompx_hold'}1">;
 def err_omp_map_type_missing : Error<
   "missing map type">;
 def err_omp_map_type_modifier_missing : Error<
   "missing map type modifier">;
 def err_omp_declare_simd_inbranch_notinbranch : Error<
   "unexpected '%0' clause, '%1' is specified already">;
+def err_omp_expected_clause_argument
+    : Error<"expected '%0' clause with an argument on '#pragma omp %1' "
+            "construct">;
 def err_expected_end_declare_target_or_variant : Error<
   "expected '#pragma omp end declare %select{target|variant}0'">;
 def err_expected_begin_declare_variant
     : Error<"'#pragma omp end declare variant' with no matching '#pragma omp "
             "begin declare variant'">;
 def err_expected_begin_assumes
     : Error<"'#pragma omp end assumes' with no matching '#pragma omp begin assumes'">;
 def warn_omp_unknown_assumption_clause_missing_id

clang/lib/Sema/SemaOpenMP.cpp

return new (Context)
       OMPUseClause(InteropVar, StartLoc, LParenLoc, VarLoc, EndLoc);
 }
 
 OMPClause *Sema::ActOnOpenMPDestroyClause(Expr *InteropVar,
                                           SourceLocation StartLoc,
                                           SourceLocation LParenLoc,
                                           SourceLocation VarLoc,
                                           SourceLocation EndLoc) {
+  if (!InteropVar && LangOpts.OpenMP >= 52 &&
+      DSAStack->getCurrentDirective() == OMPD_depobj) {
+    Diag(StartLoc, diag::err_omp_expected_clause_argument)
+        << getOpenMPClauseName(OMPC_destroy)
+        << getOpenMPDirectiveName(OMPD_depobj);
+    return nullptr;
+  }
   if (InteropVar &&
       !isValidInteropVariable(*this, InteropVar, VarLoc, OMPC_destroy))
     return nullptr;
 
   return new (Context)
       OMPDestroyClause(InteropVar, StartLoc, LParenLoc, VarLoc, EndLoc);
 }
 

clang/test/OpenMP/depobj_messages.cpp

 // RUN: %clang_cc1 -verify=expected,omp50 -fopenmp -fopenmp-version=50 \
 // RUN: -ferror-limit 100 %s -Wuninitialized
 // RUN: %clang_cc1 -verify=expected,omp51 -fopenmp -fopenmp-version=51 \
 // RUN: -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,omp52 -fopenmp -fopenmp-version=52 \
+// RUN: -ferror-limit 100 %s -Wuninitialized
 
 // RUN: %clang_cc1 -verify=expected,omp50 -fopenmp-simd -fopenmp-version=50 \
 // RUN: -ferror-limit 100 %s -Wuninitialized
 // RUN: %clang_cc1 -verify=expected,omp51 -fopenmp-simd -fopenmp-version=51 \
 // RUN: -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,omp52 -fopenmp-simd -fopenmp-version=52 \
+// RUN: -ferror-limit 100 %s -Wuninitialized
 
 struct S1 { // expected-note 2 {{declared here}}
   int a;
 } s;
 
 #pragma omp depobj(0) depend(in:s) // expected-error {{unexpected OpenMP directive '#pragma omp depobj'}}
 void foo() {
 #pragma omp depobj(0) depend(in:s) // expected-error {{'omp_depend_t' type not found; include <omp.h>}} expected-error {{expected lvalue expression}}}
 }
 
 typedef void *omp_depend_t;
 
 template <class T>
 T tmain(T argc) {
   omp_depend_t x;
 #pragma omp depobj() allocate(argc) // expected-error {{expected expression}} expected-error {{expected depobj expression}} expected-error {{unexpected OpenMP clause 'allocate' in directive '#pragma omp depobj'}}
   ;
 #pragma omp depobj(x) untied  // expected-error {{unexpected OpenMP clause 'untied' in directive '#pragma omp depobj'}}
 #pragma omp depobj(x) unknown // expected-warning {{extra tokens at the end of '#pragma omp depobj' are ignored}}
   if (argc)
-#pragma omp depobj(x) destroy // expected-error {{'#pragma omp depobj' cannot be an immediate substatement}}
+#pragma omp depobj(x) destroy // omp52-error {{expected 'destroy' clause with an argument on '#pragma omp depobj' construct}} expected-error {{'#pragma omp depobj' cannot be an immediate substatement}}
     if (argc) {
 #pragma omp depobj(x) depend(in:s)
     }
   while (argc)
 #pragma omp depobj(x)update(inout) // expected-error {{'#pragma omp depobj' cannot be an immediate substatement}}
     while (argc) {
 #pragma omp depobj(x) depend(in:s)
     }

 #pragma omp depobj(argc                          // expected-error {{expected ')'}} expected-note {{to match this '('}} expected-error {{expected lvalue expression of 'omp_depend_t' type, not 'int'}}
 #pragma omp depobj(argc,                         // expected-error {{expected ')'}} expected-note {{to match this '('}} expected-error {{expected lvalue expression of 'omp_depend_t' type, not 'int'}}
 #pragma omp depobj(argc)                         // expected-error {{expected lvalue expression of 'omp_depend_t' type, not 'int'}}
 #pragma omp depobj(S1) // expected-error {{'S1' does not refer to a value}} expected-error {{expected depobj expression}}
 #pragma omp depobj(argc) depobj(argc) // expected-warning {{extra tokens at the end of '#pragma omp depobj' are ignored}} expected-error {{expected lvalue expression of 'omp_depend_t' type, not 'int'}}
 #pragma omp parallel depobj(argc) // expected-warning {{extra tokens at the end of '#pragma omp parallel' are ignored}}
   ;
 #pragma omp depobj(x) seq_cst // expected-error {{unexpected OpenMP clause 'seq_cst' in directive '#pragma omp depobj'}}
-#pragma omp depobj(x) depend(source: x) // omp51-error {{expected depend modifier(iterator) or 'in', 'out', 'inout', 'mutexinoutset' or 'inoutset' in OpenMP clause 'depend'}} omp50-error {{expected depend modifier(iterator) or 'in', 'out', 'inout' or 'mutexinoutset' in OpenMP clause 'depend'}}
+#pragma omp depobj(x) depend(source: x) // omp52-error {{expected depend modifier(iterator) or 'in', 'out', 'inout', 'mutexinoutset' or 'inoutset' in OpenMP clause 'depend'}} omp51-error {{expected depend modifier(iterator) or 'in', 'out', 'inout', 'mutexinoutset' or 'inoutset' in OpenMP clause 'depend'}} omp50-error {{expected depend modifier(iterator) or 'in', 'out', 'inout' or 'mutexinoutset' in OpenMP clause 'depend'}}
 #pragma omp depobj(x) update // expected-error {{expected '(' after 'update'}}
-#pragma omp depobj(x) update( // expected-error {{expected ')'}} expected-note {{to match this '('}} omp51-error {{expected 'in', 'out', 'inout', 'mutexinoutset' or 'inoutset' in OpenMP clause 'update'}} omp50-error {{expected 'in', 'out', 'inout' or 'mutexinoutset' in OpenMP clause 'update'}}
-#pragma omp depobj(x) update(sink // expected-error {{expected ')'}} expected-note {{to match this '('}} omp51-error {{expected 'in', 'out', 'inout', 'mutexinoutset' or 'inoutset' in OpenMP clause 'update'}} omp50-error {{expected 'in', 'out', 'inout' or 'mutexinoutset' in OpenMP clause 'update'}}
-#pragma omp depobj(x) destroy destroy // expected-error {{directive '#pragma omp depobj' cannot contain more than one 'destroy' clause}}
+#pragma omp depobj(x) update( // expected-error {{expected ')'}} expected-note {{to match this '('}} omp52-error {{expected 'in', 'out', 'inout', 'mutexinoutset' or 'inoutset' in OpenMP clause 'update'}} omp51-error {{expected 'in', 'out', 'inout', 'mutexinoutset' or 'inoutset' in OpenMP clause 'update'}} omp50-error {{expected 'in', 'out', 'inout' or 'mutexinoutset' in OpenMP clause 'update'}}
+#pragma omp depobj(x) update(sink // expected-error {{expected ')'}} expected-note {{to match this '('}} omp52-error {{expected 'in', 'out', 'inout', 'mutexinoutset' or 'inoutset' in OpenMP clause 'update'}} omp51-error {{expected 'in', 'out', 'inout', 'mutexinoutset' or 'inoutset' in OpenMP clause 'update'}} omp50-error {{expected 'in', 'out', 'inout' or 'mutexinoutset' in OpenMP clause 'update'}}
+#pragma omp depobj(x) destroy destroy //omp52-error {{expected 'destroy' clause with an argument on '#pragma omp depobj' construct}} omp50-error {{directive '#pragma omp depobj' cannot contain more than one 'destroy' clause}} // omp52-error {{directive '#pragma omp depobj' cannot contain more than one 'destroy' clause}} omp52-error {{expected 'destroy' clause with an argument on '#pragma omp depobj' construct}} omp51-error {{directive '#pragma omp depobj' cannot contain more than one 'destroy' clause}}
 #pragma omp depobj(x) update(in) update(in) // expected-error {{directive '#pragma omp depobj' cannot contain more than one 'update' clause}}
-#pragma omp depobj(x) depend(in: argc) destroy // expected-error {{exactly one of 'depend', 'destroy', or 'update' clauses is expected}}
-#pragma omp depobj(x) destroy depend(in: argc) // expected-error {{exactly one of 'depend', 'destroy', or 'update' clauses is expected}}
+#pragma omp depobj(x) depend(in: argc) destroy // omp52-error {{expected 'destroy' clause with an argument on '#pragma omp depobj' construct}} omp51-error {{exactly one of 'depend', 'destroy', or 'update' clauses is expected}} omp50-error {{exactly one of 'depend', 'destroy', or 'update' clauses is expected}}
+#pragma omp depobj(x) destroy depend(in: argc) // omp52-error {{expected 'destroy' clause with an argument on '#pragma omp depobj' construct}} omp51-error {{exactly one of 'depend', 'destroy', or 'update' clauses is expected}} omp50-error {{exactly one of 'depend', 'destroy', or 'update' clauses is expected}}
 #pragma omp depobj(x) depend(in: argc) update(mutexinoutset) // expected-error {{exactly one of 'depend', 'destroy', or 'update' clauses is expected}}
-#pragma omp depobj(x) update(inout) destroy // expected-error {{exactly one of 'depend', 'destroy', or 'update' clauses is expected}}
+#pragma omp depobj(x) update(inout) destroy // omp52-error {{expected 'destroy' clause with an argument on '#pragma omp depobj' construct}} omp51-error {{exactly one of 'depend', 'destroy', or 'update' clauses is expected}} omp50-error {{exactly one of 'depend', 'destroy', or 'update' clauses is expected}}
 #pragma omp depobj(x) (x) depend(in: x) // expected-warning {{extra tokens at the end of '#pragma omp depobj' are ignored}}
 #pragma omp depobj(x) (x) update(in) // expected-warning {{extra tokens at the end of '#pragma omp depobj' are ignored}}
 #pragma omp depobj(x) depend(in: argc) depend(out:argc) // expected-error {{exactly one of 'depend', 'destroy', or 'update' clauses is expected}}
 #pragma omp depend(out:x) depobj(x) // expected-error {{expected an OpenMP directive}}
 #pragma omp destroy depobj(x) // expected-error {{expected an OpenMP directive}}
 #pragma omp update(out) depobj(x) // expected-error {{expected an OpenMP directive}}
 #pragma omp depobj depend(in:x) (x) // expected-error {{expected depobj expression}} expected-warning {{extra tokens at the end of '#pragma omp depobj' are ignored}} expected-error {{expected addressable lvalue expression, array element, array section or array shaping expression of non 'omp_depend_t' type}}
-#pragma omp depobj destroy (x) // expected-error {{expected depobj expression}} expected-warning {{extra tokens at the end of '#pragma omp depobj' are ignored}}
+#pragma omp depobj destroy (x) // omp52-error {{expected 'destroy' clause with an argument on '#pragma omp depobj' construct}} expected-error {{expected depobj expression}} expected-warning {{extra tokens at the end of '#pragma omp depobj' are ignored}}
 #pragma omp depobj update(in) (x) // expected-error {{expected depobj expression}} expected-warning {{extra tokens at the end of '#pragma omp depobj' are ignored}}
   return tmain(argc); // expected-note {{in instantiation of function template specialization 'tmain<int>' requested here}}
 }