[PATCH] D127855: [OpenMP] Basic parse and sema support for modifiers in order clause

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.


================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:10634
   "OpenMP constructs may not be nested inside an atomic region">;
+def err_omp_prohibited_region_order
+    : Error<"construct %0 not allowed in a region associated with a directive "
----------------
Do you have the test for this error message?


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:875
+  /// false - otherwise.
+  bool HasOrderConcurrent() const {
+    if (const SharingMapTy *Top = getTopOfStackOrNull())
----------------
`isOrderConcurrent`


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:882
+  /// 'order' clause), false - otherwise.
+  bool ParentHasOrderConcurrent() const {
+    if (const SharingMapTy *Parent = getSecondOnStackOrNull())
----------------
`isParentOrderConcurrent`


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:7215
 
+  if (this->LangOpts.OpenMP >= 51 && CalleeFnDecl->getIdentifier() &&
+      CalleeFnDecl->getName().startswith_insensitive(StringRef("omp_"))) {
----------------
Remove `this`


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:7216
+  if (this->LangOpts.OpenMP >= 51 && CalleeFnDecl->getIdentifier() &&
+      CalleeFnDecl->getName().startswith_insensitive(StringRef("omp_"))) {
+    // checking for any calls inside an Order region
----------------
I think you can drop call of `StringRef` constructor here, just `omp_`


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:16694
+  if (Kind != OMPC_ORDER_concurrent ||
+      (this->LangOpts.OpenMP < 51 && MLoc.isValid())) {
+    // Kind should be concurrent,
----------------
Remove `this`


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:16707
   }
-  return new (Context)
-      OMPOrderClause(Kind, KindKwLoc, StartLoc, LParenLoc, EndLoc);
+  if (this->LangOpts.OpenMP >= 51) {
+    if (Modifier == OMPC_ORDER_MODIFIER_unknown && MLoc.isValid()) {
----------------
Remove `this`


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D127855/new/

https://reviews.llvm.org/D127855