Skip to content

[flang][OpenMP] Rename enum OmpxHold to Ompx_Hold in parser #113366

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 22, 2024

Conversation

kparzysz
Copy link
Contributor

The convention is to use enum names that match the source spelling (up to upper/lower case), including names with underscores.

Remove the special case from unparser, update tests.

The convention is to use enum names that match the source spelling
(up to upper/lower case), including names with underscores.

Remove the special case from unparser, update tests.
@llvmbot llvmbot added flang Flang issues not falling into any other category flang:fir-hlfir flang:openmp flang:parser labels Oct 22, 2024
@llvmbot
Copy link
Member

llvmbot commented Oct 22, 2024

@llvm/pr-subscribers-flang-parser

@llvm/pr-subscribers-flang-openmp

Author: Krzysztof Parzyszek (kparzysz)

Changes

The convention is to use enum names that match the source spelling (up to upper/lower case), including names with underscores.

Remove the special case from unparser, update tests.


Full diff: https://github.com/llvm/llvm-project/pull/113366.diff

5 Files Affected:

  • (modified) flang/include/flang/Parser/parse-tree.h (+1-1)
  • (modified) flang/lib/Lower/OpenMP/Clauses.cpp (+4-4)
  • (modified) flang/lib/Parser/openmp-parsers.cpp (+1-1)
  • (modified) flang/lib/Parser/unparse.cpp (+1-7)
  • (modified) flang/test/Parser/OpenMP/map-modifiers.f90 (+4-4)
diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h
index 4a3c992c4ec51b..358fb491b84fb0 100644
--- a/flang/include/flang/Parser/parse-tree.h
+++ b/flang/include/flang/Parser/parse-tree.h
@@ -3455,7 +3455,7 @@ WRAPPER_CLASS(OmpObjectList, std::list<OmpObject>);
 // map-type-modifier -> ALWAYS | CLOSE | PRESENT | OMPX_HOLD
 // map-type -> TO | FROM | TOFROM | ALLOC | RELEASE | DELETE
 struct OmpMapClause {
-  ENUM_CLASS(TypeModifier, Always, Close, Present, OmpxHold);
+  ENUM_CLASS(TypeModifier, Always, Close, Present, Ompx_Hold);
   ENUM_CLASS(Type, To, From, Tofrom, Alloc, Release, Delete)
   TUPLE_CLASS_BOILERPLATE(OmpMapClause);
   std::tuple<std::optional<std::list<TypeModifier>>, std::optional<Type>,
diff --git a/flang/lib/Lower/OpenMP/Clauses.cpp b/flang/lib/Lower/OpenMP/Clauses.cpp
index 64d661256a187e..303b9ac5b7f4d5 100644
--- a/flang/lib/Lower/OpenMP/Clauses.cpp
+++ b/flang/lib/Lower/OpenMP/Clauses.cpp
@@ -843,10 +843,10 @@ Map make(const parser::OmpClause::Map &inp,
   CLAUSET_ENUM_CONVERT( //
       convert2, parser::OmpMapClause::TypeModifier, Map::MapTypeModifier,
       // clang-format off
-      MS(Always,   Always)
-      MS(Close,    Close)
-      MS(OmpxHold, OmpxHold)
-      MS(Present,  Present)
+      MS(Always,    Always)
+      MS(Close,     Close)
+      MS(Ompx_Hold, OmpxHold)
+      MS(Present,   Present)
       // clang-format on
   );
 
diff --git a/flang/lib/Parser/openmp-parsers.cpp b/flang/lib/Parser/openmp-parsers.cpp
index 52c7529369dfb5..9b4cdf3720b788 100644
--- a/flang/lib/Parser/openmp-parsers.cpp
+++ b/flang/lib/Parser/openmp-parsers.cpp
@@ -110,7 +110,7 @@ TYPE_PARSER(construct<OmpProcBindClause>(
 TYPE_PARSER(construct<OmpMapClause::TypeModifier>(
     "ALWAYS" >> pure(OmpMapClause::TypeModifier::Always) ||
     "CLOSE" >> pure(OmpMapClause::TypeModifier::Close) ||
-    "OMPX_HOLD" >> pure(OmpMapClause::TypeModifier::OmpxHold) ||
+    "OMPX_HOLD" >> pure(OmpMapClause::TypeModifier::Ompx_Hold) ||
     "PRESENT" >> pure(OmpMapClause::TypeModifier::Present)))
 
 TYPE_PARSER(
diff --git a/flang/lib/Parser/unparse.cpp b/flang/lib/Parser/unparse.cpp
index d1011fe58a0264..d65ddfabb05d9e 100644
--- a/flang/lib/Parser/unparse.cpp
+++ b/flang/lib/Parser/unparse.cpp
@@ -2087,13 +2087,6 @@ class UnparseVisitor {
     }
     Walk(std::get<OmpObjectList>(x.t));
   }
-  void Unparse(const OmpMapClause::TypeModifier &x) {
-    if (x == OmpMapClause::TypeModifier::OmpxHold) {
-      Word("OMPX_HOLD");
-    } else {
-      Word(OmpMapClause::EnumToString(x));
-    }
-  }
   void Unparse(const OmpScheduleModifier &x) {
     Walk(std::get<OmpScheduleModifier::Modifier1>(x.t));
     Walk(",", std::get<std::optional<OmpScheduleModifier::Modifier2>>(x.t));
@@ -2801,6 +2794,7 @@ class UnparseVisitor {
   WALK_NESTED_ENUM(OmpOrderClause, Type) // OMP order-type
   WALK_NESTED_ENUM(OmpOrderModifier, Kind) // OMP order-modifier
   WALK_NESTED_ENUM(OmpMapClause, Type) // OMP map-type
+  WALK_NESTED_ENUM(OmpMapClause, TypeModifier) // OMP map-type-modifier
 #undef WALK_NESTED_ENUM
   void Unparse(const ReductionOperator::Operator x) {
     switch (x) {
diff --git a/flang/test/Parser/OpenMP/map-modifiers.f90 b/flang/test/Parser/OpenMP/map-modifiers.f90
index 4c6dcde5a20c5d..895e29f9101a51 100644
--- a/flang/test/Parser/OpenMP/map-modifiers.f90
+++ b/flang/test/Parser/OpenMP/map-modifiers.f90
@@ -18,7 +18,7 @@ subroutine f00(x)
 !PARSE-TREE: OmpBeginBlockDirective
 !PARSE-TREE: | OmpBlockDirective -> llvm::omp::Directive = target
 !PARSE-TREE: | OmpClauseList -> OmpClause -> Map -> OmpMapClause
-!PARSE-TREE: | | TypeModifier = OmpxHold
+!PARSE-TREE: | | TypeModifier = Ompx_Hold
 !PARSE-TREE: | | TypeModifier = Always
 !PARSE-TREE: | | TypeModifier = Present
 !PARSE-TREE: | | TypeModifier = Close
@@ -42,7 +42,7 @@ subroutine f01(x)
 !PARSE-TREE: OmpBeginBlockDirective
 !PARSE-TREE: | OmpBlockDirective -> llvm::omp::Directive = target
 !PARSE-TREE: | OmpClauseList -> OmpClause -> Map -> OmpMapClause
-!PARSE-TREE: | | TypeModifier = OmpxHold
+!PARSE-TREE: | | TypeModifier = Ompx_Hold
 !PARSE-TREE: | | TypeModifier = Always
 !PARSE-TREE: | | TypeModifier = Present
 !PARSE-TREE: | | TypeModifier = Close
@@ -104,7 +104,7 @@ subroutine f10(x)
 !PARSE-TREE: OmpBeginBlockDirective
 !PARSE-TREE: | OmpBlockDirective -> llvm::omp::Directive = target
 !PARSE-TREE: | OmpClauseList -> OmpClause -> Map -> OmpMapClause
-!PARSE-TREE: | | TypeModifier = OmpxHold
+!PARSE-TREE: | | TypeModifier = Ompx_Hold
 !PARSE-TREE: | | TypeModifier = Always
 !PARSE-TREE: | | TypeModifier = Present
 !PARSE-TREE: | | TypeModifier = Close
@@ -128,7 +128,7 @@ subroutine f11(x)
 !PARSE-TREE: OmpBeginBlockDirective
 !PARSE-TREE: | OmpBlockDirective -> llvm::omp::Directive = target
 !PARSE-TREE: | OmpClauseList -> OmpClause -> Map -> OmpMapClause
-!PARSE-TREE: | | TypeModifier = OmpxHold
+!PARSE-TREE: | | TypeModifier = Ompx_Hold
 !PARSE-TREE: | | TypeModifier = Always
 !PARSE-TREE: | | TypeModifier = Present
 !PARSE-TREE: | | TypeModifier = Close

@llvmbot
Copy link
Member

llvmbot commented Oct 22, 2024

@llvm/pr-subscribers-flang-fir-hlfir

Author: Krzysztof Parzyszek (kparzysz)

Changes

The convention is to use enum names that match the source spelling (up to upper/lower case), including names with underscores.

Remove the special case from unparser, update tests.


Full diff: https://github.com/llvm/llvm-project/pull/113366.diff

5 Files Affected:

  • (modified) flang/include/flang/Parser/parse-tree.h (+1-1)
  • (modified) flang/lib/Lower/OpenMP/Clauses.cpp (+4-4)
  • (modified) flang/lib/Parser/openmp-parsers.cpp (+1-1)
  • (modified) flang/lib/Parser/unparse.cpp (+1-7)
  • (modified) flang/test/Parser/OpenMP/map-modifiers.f90 (+4-4)
diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h
index 4a3c992c4ec51b..358fb491b84fb0 100644
--- a/flang/include/flang/Parser/parse-tree.h
+++ b/flang/include/flang/Parser/parse-tree.h
@@ -3455,7 +3455,7 @@ WRAPPER_CLASS(OmpObjectList, std::list<OmpObject>);
 // map-type-modifier -> ALWAYS | CLOSE | PRESENT | OMPX_HOLD
 // map-type -> TO | FROM | TOFROM | ALLOC | RELEASE | DELETE
 struct OmpMapClause {
-  ENUM_CLASS(TypeModifier, Always, Close, Present, OmpxHold);
+  ENUM_CLASS(TypeModifier, Always, Close, Present, Ompx_Hold);
   ENUM_CLASS(Type, To, From, Tofrom, Alloc, Release, Delete)
   TUPLE_CLASS_BOILERPLATE(OmpMapClause);
   std::tuple<std::optional<std::list<TypeModifier>>, std::optional<Type>,
diff --git a/flang/lib/Lower/OpenMP/Clauses.cpp b/flang/lib/Lower/OpenMP/Clauses.cpp
index 64d661256a187e..303b9ac5b7f4d5 100644
--- a/flang/lib/Lower/OpenMP/Clauses.cpp
+++ b/flang/lib/Lower/OpenMP/Clauses.cpp
@@ -843,10 +843,10 @@ Map make(const parser::OmpClause::Map &inp,
   CLAUSET_ENUM_CONVERT( //
       convert2, parser::OmpMapClause::TypeModifier, Map::MapTypeModifier,
       // clang-format off
-      MS(Always,   Always)
-      MS(Close,    Close)
-      MS(OmpxHold, OmpxHold)
-      MS(Present,  Present)
+      MS(Always,    Always)
+      MS(Close,     Close)
+      MS(Ompx_Hold, OmpxHold)
+      MS(Present,   Present)
       // clang-format on
   );
 
diff --git a/flang/lib/Parser/openmp-parsers.cpp b/flang/lib/Parser/openmp-parsers.cpp
index 52c7529369dfb5..9b4cdf3720b788 100644
--- a/flang/lib/Parser/openmp-parsers.cpp
+++ b/flang/lib/Parser/openmp-parsers.cpp
@@ -110,7 +110,7 @@ TYPE_PARSER(construct<OmpProcBindClause>(
 TYPE_PARSER(construct<OmpMapClause::TypeModifier>(
     "ALWAYS" >> pure(OmpMapClause::TypeModifier::Always) ||
     "CLOSE" >> pure(OmpMapClause::TypeModifier::Close) ||
-    "OMPX_HOLD" >> pure(OmpMapClause::TypeModifier::OmpxHold) ||
+    "OMPX_HOLD" >> pure(OmpMapClause::TypeModifier::Ompx_Hold) ||
     "PRESENT" >> pure(OmpMapClause::TypeModifier::Present)))
 
 TYPE_PARSER(
diff --git a/flang/lib/Parser/unparse.cpp b/flang/lib/Parser/unparse.cpp
index d1011fe58a0264..d65ddfabb05d9e 100644
--- a/flang/lib/Parser/unparse.cpp
+++ b/flang/lib/Parser/unparse.cpp
@@ -2087,13 +2087,6 @@ class UnparseVisitor {
     }
     Walk(std::get<OmpObjectList>(x.t));
   }
-  void Unparse(const OmpMapClause::TypeModifier &x) {
-    if (x == OmpMapClause::TypeModifier::OmpxHold) {
-      Word("OMPX_HOLD");
-    } else {
-      Word(OmpMapClause::EnumToString(x));
-    }
-  }
   void Unparse(const OmpScheduleModifier &x) {
     Walk(std::get<OmpScheduleModifier::Modifier1>(x.t));
     Walk(",", std::get<std::optional<OmpScheduleModifier::Modifier2>>(x.t));
@@ -2801,6 +2794,7 @@ class UnparseVisitor {
   WALK_NESTED_ENUM(OmpOrderClause, Type) // OMP order-type
   WALK_NESTED_ENUM(OmpOrderModifier, Kind) // OMP order-modifier
   WALK_NESTED_ENUM(OmpMapClause, Type) // OMP map-type
+  WALK_NESTED_ENUM(OmpMapClause, TypeModifier) // OMP map-type-modifier
 #undef WALK_NESTED_ENUM
   void Unparse(const ReductionOperator::Operator x) {
     switch (x) {
diff --git a/flang/test/Parser/OpenMP/map-modifiers.f90 b/flang/test/Parser/OpenMP/map-modifiers.f90
index 4c6dcde5a20c5d..895e29f9101a51 100644
--- a/flang/test/Parser/OpenMP/map-modifiers.f90
+++ b/flang/test/Parser/OpenMP/map-modifiers.f90
@@ -18,7 +18,7 @@ subroutine f00(x)
 !PARSE-TREE: OmpBeginBlockDirective
 !PARSE-TREE: | OmpBlockDirective -> llvm::omp::Directive = target
 !PARSE-TREE: | OmpClauseList -> OmpClause -> Map -> OmpMapClause
-!PARSE-TREE: | | TypeModifier = OmpxHold
+!PARSE-TREE: | | TypeModifier = Ompx_Hold
 !PARSE-TREE: | | TypeModifier = Always
 !PARSE-TREE: | | TypeModifier = Present
 !PARSE-TREE: | | TypeModifier = Close
@@ -42,7 +42,7 @@ subroutine f01(x)
 !PARSE-TREE: OmpBeginBlockDirective
 !PARSE-TREE: | OmpBlockDirective -> llvm::omp::Directive = target
 !PARSE-TREE: | OmpClauseList -> OmpClause -> Map -> OmpMapClause
-!PARSE-TREE: | | TypeModifier = OmpxHold
+!PARSE-TREE: | | TypeModifier = Ompx_Hold
 !PARSE-TREE: | | TypeModifier = Always
 !PARSE-TREE: | | TypeModifier = Present
 !PARSE-TREE: | | TypeModifier = Close
@@ -104,7 +104,7 @@ subroutine f10(x)
 !PARSE-TREE: OmpBeginBlockDirective
 !PARSE-TREE: | OmpBlockDirective -> llvm::omp::Directive = target
 !PARSE-TREE: | OmpClauseList -> OmpClause -> Map -> OmpMapClause
-!PARSE-TREE: | | TypeModifier = OmpxHold
+!PARSE-TREE: | | TypeModifier = Ompx_Hold
 !PARSE-TREE: | | TypeModifier = Always
 !PARSE-TREE: | | TypeModifier = Present
 !PARSE-TREE: | | TypeModifier = Close
@@ -128,7 +128,7 @@ subroutine f11(x)
 !PARSE-TREE: OmpBeginBlockDirective
 !PARSE-TREE: | OmpBlockDirective -> llvm::omp::Directive = target
 !PARSE-TREE: | OmpClauseList -> OmpClause -> Map -> OmpMapClause
-!PARSE-TREE: | | TypeModifier = OmpxHold
+!PARSE-TREE: | | TypeModifier = Ompx_Hold
 !PARSE-TREE: | | TypeModifier = Always
 !PARSE-TREE: | | TypeModifier = Present
 !PARSE-TREE: | | TypeModifier = Close

Copy link
Contributor

@kiranchandramohan kiranchandramohan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LG. Thanks for splitting this out.

@kparzysz kparzysz merged commit a8d506b into llvm:main Oct 22, 2024
13 checks passed
@kparzysz kparzysz deleted the users/kparzysz/enum-name-ompxclause branch October 22, 2024 21:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flang:fir-hlfir flang:openmp flang:parser flang Flang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants