[flang-commits] [flang] [flang][OpenMP] Make OmpDirectiveNameModifier a distrinct type (PR #150768)

[Krzysztof Parzyszek via flang-commits]

https://github.com/kparzysz updated https://github.com/llvm/llvm-project/pull/150768

>From 66c18fa8a408d6ec0123f901566a03159d2eb593 Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Sat, 26 Jul 2025 09:47:03 -0500
Subject: [PATCH 1/3] [flang][OpenMP] Make OmpDirectiveNameModifier a distrinct
 type

It was an alias for OmpDirectiveName, which could cause confusion in
parse-tree visitors: a visitor for OmpDirectiveNameModifier could be
executed for an OmpDirectiveName node, leading to unexpected results.
---
 flang/include/flang/Parser/parse-tree.h | 12 +++++++++++-
 flang/lib/Parser/openmp-parsers.cpp     |  5 ++++-
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h
index 3a28f6f9731c3..0fb66c9f683e4 100644
--- a/flang/include/flang/Parser/parse-tree.h
+++ b/flang/include/flang/Parser/parse-tree.h
@@ -3469,6 +3469,12 @@ WRAPPER_CLASS(PauseStmt, std::optional<StopCode>);
 
 // --- Common definitions
 
+#define INHERITED_WRAPPER_CLASS_BOILERPLATE(classname, basename) \
+  BOILERPLATE(classname); \
+  classname(decltype(basename::v) &&x) : basename(std::move(x)) {} \
+  classname(basename &&base) : basename(std::move(base)) {} \
+  using WrapperTrait = std::true_type
+
 struct OmpClause;
 struct OmpDirectiveSpecification;
 
@@ -3476,6 +3482,7 @@ struct OmpDirectiveName {
   // No boilerplates: this class should be copyable, movable, etc.
   constexpr OmpDirectiveName() = default;
   constexpr OmpDirectiveName(const OmpDirectiveName &) = default;
+  constexpr OmpDirectiveName(llvm::omp::Directive x) : v(x) {}
   // Construct from an already parsed text. Use Verbatim for this because
   // Verbatim's source corresponds to an actual source location.
   // This allows "construct<OmpDirectiveName>(Verbatim("<name>"))".
@@ -3848,7 +3855,10 @@ struct OmpDeviceModifier {
 // [*] The IF clause is allowed on CANCEL in OpenMP 4.5, but only without
 // the directive-name-modifier. For the sake of uniformity CANCEL can be
 // considered a valid value in 4.5 as well.
-using OmpDirectiveNameModifier = OmpDirectiveName;
+struct OmpDirectiveNameModifier : public OmpDirectiveName {
+  INHERITED_WRAPPER_CLASS_BOILERPLATE(
+      OmpDirectiveNameModifier, OmpDirectiveName);
+};
 
 // Ref: [5.1:205-209], [5.2:166-168]
 //
diff --git a/flang/lib/Parser/openmp-parsers.cpp b/flang/lib/Parser/openmp-parsers.cpp
index 1c626148a03ae..9abec5aef8329 100644
--- a/flang/lib/Parser/openmp-parsers.cpp
+++ b/flang/lib/Parser/openmp-parsers.cpp
@@ -466,6 +466,8 @@ TYPE_PARSER(construct<OmpDeviceModifier>(
     "ANCESTOR" >> pure(OmpDeviceModifier::Value::Ancestor) ||
     "DEVICE_NUM" >> pure(OmpDeviceModifier::Value::Device_Num)))
 
+TYPE_PARSER(construct<OmpDirectiveNameModifier>(OmpDirectiveNameParser{}))
+
 TYPE_PARSER(construct<OmpExpectation>( //
     "PRESENT" >> pure(OmpExpectation::Value::Present)))
 
@@ -609,7 +611,8 @@ TYPE_PARSER(sourced(construct<OmpFromClause::Modifier>(
 TYPE_PARSER(sourced(
     construct<OmpGrainsizeClause::Modifier>(Parser<OmpPrescriptiveness>{})))
 
-TYPE_PARSER(sourced(construct<OmpIfClause::Modifier>(OmpDirectiveNameParser{})))
+TYPE_PARSER(sourced(
+    construct<OmpIfClause::Modifier>(Parser<OmpDirectiveNameModifier>{})))
 
 TYPE_PARSER(sourced(
     construct<OmpInitClause::Modifier>(

>From 14dc21327817f681c7b62cb54fa5ceafb6ca9788 Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Sat, 26 Jul 2025 10:08:46 -0500
Subject: [PATCH 2/3] fix test

---
 flang/test/Examples/omp-atomic.f90 | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/flang/test/Examples/omp-atomic.f90 b/flang/test/Examples/omp-atomic.f90
index 934f84f132484..5695b621e4bff 100644
--- a/flang/test/Examples/omp-atomic.f90
+++ b/flang/test/Examples/omp-atomic.f90
@@ -31,13 +31,13 @@
 ! CHECK-NEXT:    - clause:      read
 ! CHECK-NEXT:      details:     ''
 ! CHECK-NEXT:    - clause:      seq_cst
-! CHECK-NEXT:      details:     'name_modifier=atomic;'
+! CHECK-NEXT:      details:     ''
 ! CHECK-NEXT:- file:            '{{[^"]*}}omp-atomic.f90'
 ! CHECK-NEXT:  line:            12
 ! CHECK-NEXT:  construct:       atomic
 ! CHECK-NEXT:  clauses:
 ! CHECK-NEXT:    - clause:      seq_cst
-! CHECK-NEXT:      details:     'name_modifier=atomic;'
+! CHECK-NEXT:      details:     ''
 ! CHECK-NEXT:    - clause:      write
 ! CHECK-NEXT:      details:     ''
 ! CHECK-NEXT:- file:            '{{[^"]*}}omp-atomic.f90'
@@ -45,7 +45,7 @@
 ! CHECK-NEXT:  construct:       atomic
 ! CHECK-NEXT:  clauses:
 ! CHECK-NEXT:    - clause:      capture
-! CHECK-NEXT:      details:     'name_modifier=atomic;name_modifier=atomic;'
+! CHECK-NEXT:      details:     ''
 ! CHECK-NEXT:    - clause:      seq_cst
 ! CHECK-NEXT:      details:     ''
 ! CHECK-NEXT:- file:            '{{[^"]*}}omp-atomic.f90'

>From 8a16cdceadf8234183e74f3d91218d56b1302987 Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Wed, 30 Jul 2025 09:48:27 -0500
Subject: [PATCH 3/3] Inherit constructors

---
 flang/include/flang/Parser/parse-tree.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h
index 0fb66c9f683e4..4c14f42309194 100644
--- a/flang/include/flang/Parser/parse-tree.h
+++ b/flang/include/flang/Parser/parse-tree.h
@@ -3471,7 +3471,7 @@ WRAPPER_CLASS(PauseStmt, std::optional<StopCode>);
 
 #define INHERITED_WRAPPER_CLASS_BOILERPLATE(classname, basename) \
   BOILERPLATE(classname); \
-  classname(decltype(basename::v) &&x) : basename(std::move(x)) {} \
+  using basename::basename; \
   classname(basename &&base) : basename(std::move(base)) {} \
   using WrapperTrait = std::true_type