Skip to content

Conversation

kparzysz
Copy link
Contributor

No description provided.

@kparzysz kparzysz requested review from Stylie777 and tblah September 18, 2025 14:36
@llvmbot llvmbot added flang Flang issues not falling into any other category flang:openmp flang:semantics flang:parser labels Sep 18, 2025
@llvmbot
Copy link
Member

llvmbot commented Sep 18, 2025

@llvm/pr-subscribers-flang-parser

@llvm/pr-subscribers-flang-semantics

Author: Krzysztof Parzyszek (kparzysz)

Changes

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

6 Files Affected:

  • (modified) flang/include/flang/Parser/openmp-utils.h (-8)
  • (modified) flang/include/flang/Parser/parse-tree.h (+2-6)
  • (modified) flang/lib/Parser/openmp-parsers.cpp (+22-19)
  • (modified) flang/lib/Parser/unparse.cpp (+8-3)
  • (modified) flang/lib/Semantics/check-omp-structure.cpp (+11-10)
  • (modified) flang/lib/Semantics/check-omp-structure.h (+2)
diff --git a/flang/include/flang/Parser/openmp-utils.h b/flang/include/flang/Parser/openmp-utils.h
index 032fb8996fe48..af14b20989af0 100644
--- a/flang/include/flang/Parser/openmp-utils.h
+++ b/flang/include/flang/Parser/openmp-utils.h
@@ -39,8 +39,6 @@ struct ConstructId {
   }
 
 MAKE_CONSTR_ID(OmpDeclareVariantDirective, D::OMPD_declare_variant);
-MAKE_CONSTR_ID(OmpErrorDirective, D::OMPD_error);
-MAKE_CONSTR_ID(OmpMetadirectiveDirective, D::OMPD_metadirective);
 MAKE_CONSTR_ID(OpenMPDeclarativeAllocate, D::OMPD_allocate);
 MAKE_CONSTR_ID(OpenMPDeclarativeAssumes, D::OMPD_assumes);
 MAKE_CONSTR_ID(OpenMPDeclareMapperConstruct, D::OMPD_declare_mapper);
@@ -62,10 +60,6 @@ struct DirectiveNameScope {
     return name;
   }
 
-  static OmpDirectiveName GetOmpDirectiveName(const OmpNothingDirective &x) {
-    return MakeName(x.source, llvm::omp::Directive::OMPD_nothing);
-  }
-
   static OmpDirectiveName GetOmpDirectiveName(const OmpBeginLoopDirective &x) {
     return x.DirName();
   }
@@ -102,8 +96,6 @@ struct DirectiveNameScope {
       if constexpr (std::is_base_of_v<OmpBlockConstruct, T>) {
         return std::get<OmpBeginDirective>(x.t).DirName();
       } else if constexpr (std::is_same_v<T, OmpDeclareVariantDirective> ||
-          std::is_same_v<T, OmpErrorDirective> ||
-          std::is_same_v<T, OmpMetadirectiveDirective> ||
           std::is_same_v<T, OpenMPDeclarativeAllocate> ||
           std::is_same_v<T, OpenMPDeclarativeAssumes> ||
           std::is_same_v<T, OpenMPDeclareMapperConstruct> ||
diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h
index 7307283eb91ec..eb64bc396a1b4 100644
--- a/flang/include/flang/Parser/parse-tree.h
+++ b/flang/include/flang/Parser/parse-tree.h
@@ -4837,17 +4837,13 @@ struct OmpMetadirectiveDirective {
 // nothing-directive ->
 //    NOTHING                                     // since 5.1
 struct OmpNothingDirective {
-  using EmptyTrait = std::true_type;
-  COPY_AND_ASSIGN_BOILERPLATE(OmpNothingDirective);
-  CharBlock source;
+  WRAPPER_CLASS_BOILERPLATE(OmpNothingDirective, OmpDirectiveSpecification);
 };
 
 // Ref: OpenMP [5.2:216-218]
 // ERROR AT(compilation|execution) SEVERITY(fatal|warning) MESSAGE("msg-str)
 struct OmpErrorDirective {
-  TUPLE_CLASS_BOILERPLATE(OmpErrorDirective);
-  CharBlock source;
-  std::tuple<Verbatim, OmpClauseList> t;
+  WRAPPER_CLASS_BOILERPLATE(OmpErrorDirective, OmpDirectiveSpecification);
 };
 
 struct OpenMPUtilityConstruct {
diff --git a/flang/lib/Parser/openmp-parsers.cpp b/flang/lib/Parser/openmp-parsers.cpp
index c6d4de108fb59..9d7fbf63cfa4c 100644
--- a/flang/lib/Parser/openmp-parsers.cpp
+++ b/flang/lib/Parser/openmp-parsers.cpp
@@ -1276,11 +1276,18 @@ TYPE_PARSER(sourced(construct<OmpClauseList>(
 // 2.1 (variable | /common-block/ | array-sections)
 TYPE_PARSER(construct<OmpObjectList>(nonemptyList(Parser<OmpObject>{})))
 
-TYPE_PARSER(sourced(construct<OmpErrorDirective>(
-    verbatim("ERROR"_tok), Parser<OmpClauseList>{})))
-
 // --- Parsers for directives and constructs --------------------------
 
+static inline constexpr auto IsDirective(llvm::omp::Directive dir) {
+  return [dir](const OmpDirectiveName &name) -> bool { return dir == name.v; };
+}
+
+static inline constexpr auto IsMemberOf(const DirectiveSet &dirs) {
+  return [&dirs](const OmpDirectiveName &name) -> bool {
+    return dirs.test(llvm::to_underlying(name.v));
+  };
+}
+
 TYPE_PARSER(sourced(construct<OmpDirectiveName>(OmpDirectiveNameParser{})))
 
 OmpDirectiveSpecification static makeFlushFromOldSyntax(Verbatim &&text,
@@ -1355,27 +1362,23 @@ struct LooselyStructuredBlockParser {
   }
 };
 
-TYPE_PARSER(sourced(construct<OmpNothingDirective>("NOTHING" >> ok)))
+TYPE_PARSER(construct<OmpErrorDirective>(
+    predicated(Parser<OmpDirectiveName>{},
+        IsDirective(llvm::omp::Directive::OMPD_error)) >=
+    Parser<OmpDirectiveSpecification>{}))
 
-TYPE_PARSER(sourced(construct<OpenMPUtilityConstruct>(
-    sourced(construct<OpenMPUtilityConstruct>(
-        sourced(Parser<OmpErrorDirective>{}))) ||
-    sourced(construct<OpenMPUtilityConstruct>(
-        sourced(Parser<OmpNothingDirective>{}))))))
+TYPE_PARSER(construct<OmpNothingDirective>(
+    predicated(Parser<OmpDirectiveName>{},
+        IsDirective(llvm::omp::Directive::OMPD_nothing)) >=
+    Parser<OmpDirectiveSpecification>{}))
+
+TYPE_PARSER( //
+    sourced(construct<OpenMPUtilityConstruct>(Parser<OmpErrorDirective>{})) ||
+    sourced(construct<OpenMPUtilityConstruct>(Parser<OmpNothingDirective>{})))
 
 TYPE_PARSER(sourced(construct<OmpMetadirectiveDirective>(
     verbatim("METADIRECTIVE"_tok), Parser<OmpClauseList>{})))
 
-static inline constexpr auto IsDirective(llvm::omp::Directive dir) {
-  return [dir](const OmpDirectiveName &name) -> bool { return dir == name.v; };
-}
-
-static inline constexpr auto IsMemberOf(const DirectiveSet &dirs) {
-  return [&dirs](const OmpDirectiveName &name) -> bool {
-    return dirs.test(llvm::to_underlying(name.v));
-  };
-}
-
 struct OmpBeginDirectiveParser {
   using resultType = OmpDirectiveSpecification;
 
diff --git a/flang/lib/Parser/unparse.cpp b/flang/lib/Parser/unparse.cpp
index 73bbbc04f46b1..9e455695b7b15 100644
--- a/flang/lib/Parser/unparse.cpp
+++ b/flang/lib/Parser/unparse.cpp
@@ -2610,13 +2610,18 @@ class UnparseVisitor {
     return false;
   }
   void Unparse(const OmpErrorDirective &x) {
-    Word("!$OMP ERROR ");
-    Walk(x.t);
+    BeginOpenMP();
+    Word("!$OMP ");
+    Walk(x.v);
     Put("\n");
+    EndOpenMP();
   }
   void Unparse(const OmpNothingDirective &x) {
-    Word("!$OMP NOTHING");
+    BeginOpenMP();
+    Word("!$OMP ");
+    Walk(x.v);
     Put("\n");
+    EndOpenMP();
   }
   void Unparse(const OmpSectionsDirective &x) {
     switch (x.v) {
diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index 4c7cd1734e0e7..266ef221ea7c9 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -608,14 +608,6 @@ template <typename Checker> struct DirectiveSpellingVisitor {
     checker_(GetDirName(x.t).source, Directive::OMPD_dispatch);
     return false;
   }
-  bool Pre(const parser::OmpErrorDirective &x) {
-    checker_(std::get<parser::Verbatim>(x.t).source, Directive::OMPD_error);
-    return false;
-  }
-  bool Pre(const parser::OmpNothingDirective &x) {
-    checker_(x.source, Directive::OMPD_nothing);
-    return false;
-  }
   bool Pre(const parser::OpenMPExecutableAllocate &x) {
     checker_(std::get<parser::Verbatim>(x.t).source, Directive::OMPD_allocate);
     return false;
@@ -1755,8 +1747,17 @@ void OmpStructureChecker::Leave(const parser::OpenMPDeclareTargetConstruct &x) {
 }
 
 void OmpStructureChecker::Enter(const parser::OmpErrorDirective &x) {
-  const auto &dir{std::get<parser::Verbatim>(x.t)};
-  PushContextAndClauseSets(dir.source, llvm::omp::Directive::OMPD_error);
+  const parser::OmpDirectiveName &dirName{x.v.DirName()};
+  PushContextAndClauseSets(dirName.source, dirName.v);
+}
+
+void OmpStructureChecker::Enter(const parser::OmpNothingDirective &x) {
+  const parser::OmpDirectiveName &dirName{x.v.DirName()};
+  PushContextAndClauseSets(dirName.source, dirName.v);
+}
+
+void OmpStructureChecker::Leave(const parser::OmpNothingDirective &x) {
+  dirContext_.pop_back();
 }
 
 void OmpStructureChecker::Enter(const parser::OpenMPDispatchConstruct &x) {
diff --git a/flang/lib/Semantics/check-omp-structure.h b/flang/lib/Semantics/check-omp-structure.h
index ce074f5f3f86e..81cabdfed0a39 100644
--- a/flang/lib/Semantics/check-omp-structure.h
+++ b/flang/lib/Semantics/check-omp-structure.h
@@ -120,6 +120,8 @@ class OmpStructureChecker
   void Leave(const parser::OpenMPDispatchConstruct &);
   void Enter(const parser::OmpErrorDirective &);
   void Leave(const parser::OmpErrorDirective &);
+  void Enter(const parser::OmpNothingDirective &);
+  void Leave(const parser::OmpNothingDirective &);
   void Enter(const parser::OpenMPExecutableAllocate &);
   void Leave(const parser::OpenMPExecutableAllocate &);
   void Enter(const parser::OpenMPAllocatorsConstruct &);

@llvmbot
Copy link
Member

llvmbot commented Sep 18, 2025

@llvm/pr-subscribers-flang-openmp

Author: Krzysztof Parzyszek (kparzysz)

Changes

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

6 Files Affected:

  • (modified) flang/include/flang/Parser/openmp-utils.h (-8)
  • (modified) flang/include/flang/Parser/parse-tree.h (+2-6)
  • (modified) flang/lib/Parser/openmp-parsers.cpp (+22-19)
  • (modified) flang/lib/Parser/unparse.cpp (+8-3)
  • (modified) flang/lib/Semantics/check-omp-structure.cpp (+11-10)
  • (modified) flang/lib/Semantics/check-omp-structure.h (+2)
diff --git a/flang/include/flang/Parser/openmp-utils.h b/flang/include/flang/Parser/openmp-utils.h
index 032fb8996fe48..af14b20989af0 100644
--- a/flang/include/flang/Parser/openmp-utils.h
+++ b/flang/include/flang/Parser/openmp-utils.h
@@ -39,8 +39,6 @@ struct ConstructId {
   }
 
 MAKE_CONSTR_ID(OmpDeclareVariantDirective, D::OMPD_declare_variant);
-MAKE_CONSTR_ID(OmpErrorDirective, D::OMPD_error);
-MAKE_CONSTR_ID(OmpMetadirectiveDirective, D::OMPD_metadirective);
 MAKE_CONSTR_ID(OpenMPDeclarativeAllocate, D::OMPD_allocate);
 MAKE_CONSTR_ID(OpenMPDeclarativeAssumes, D::OMPD_assumes);
 MAKE_CONSTR_ID(OpenMPDeclareMapperConstruct, D::OMPD_declare_mapper);
@@ -62,10 +60,6 @@ struct DirectiveNameScope {
     return name;
   }
 
-  static OmpDirectiveName GetOmpDirectiveName(const OmpNothingDirective &x) {
-    return MakeName(x.source, llvm::omp::Directive::OMPD_nothing);
-  }
-
   static OmpDirectiveName GetOmpDirectiveName(const OmpBeginLoopDirective &x) {
     return x.DirName();
   }
@@ -102,8 +96,6 @@ struct DirectiveNameScope {
       if constexpr (std::is_base_of_v<OmpBlockConstruct, T>) {
         return std::get<OmpBeginDirective>(x.t).DirName();
       } else if constexpr (std::is_same_v<T, OmpDeclareVariantDirective> ||
-          std::is_same_v<T, OmpErrorDirective> ||
-          std::is_same_v<T, OmpMetadirectiveDirective> ||
           std::is_same_v<T, OpenMPDeclarativeAllocate> ||
           std::is_same_v<T, OpenMPDeclarativeAssumes> ||
           std::is_same_v<T, OpenMPDeclareMapperConstruct> ||
diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h
index 7307283eb91ec..eb64bc396a1b4 100644
--- a/flang/include/flang/Parser/parse-tree.h
+++ b/flang/include/flang/Parser/parse-tree.h
@@ -4837,17 +4837,13 @@ struct OmpMetadirectiveDirective {
 // nothing-directive ->
 //    NOTHING                                     // since 5.1
 struct OmpNothingDirective {
-  using EmptyTrait = std::true_type;
-  COPY_AND_ASSIGN_BOILERPLATE(OmpNothingDirective);
-  CharBlock source;
+  WRAPPER_CLASS_BOILERPLATE(OmpNothingDirective, OmpDirectiveSpecification);
 };
 
 // Ref: OpenMP [5.2:216-218]
 // ERROR AT(compilation|execution) SEVERITY(fatal|warning) MESSAGE("msg-str)
 struct OmpErrorDirective {
-  TUPLE_CLASS_BOILERPLATE(OmpErrorDirective);
-  CharBlock source;
-  std::tuple<Verbatim, OmpClauseList> t;
+  WRAPPER_CLASS_BOILERPLATE(OmpErrorDirective, OmpDirectiveSpecification);
 };
 
 struct OpenMPUtilityConstruct {
diff --git a/flang/lib/Parser/openmp-parsers.cpp b/flang/lib/Parser/openmp-parsers.cpp
index c6d4de108fb59..9d7fbf63cfa4c 100644
--- a/flang/lib/Parser/openmp-parsers.cpp
+++ b/flang/lib/Parser/openmp-parsers.cpp
@@ -1276,11 +1276,18 @@ TYPE_PARSER(sourced(construct<OmpClauseList>(
 // 2.1 (variable | /common-block/ | array-sections)
 TYPE_PARSER(construct<OmpObjectList>(nonemptyList(Parser<OmpObject>{})))
 
-TYPE_PARSER(sourced(construct<OmpErrorDirective>(
-    verbatim("ERROR"_tok), Parser<OmpClauseList>{})))
-
 // --- Parsers for directives and constructs --------------------------
 
+static inline constexpr auto IsDirective(llvm::omp::Directive dir) {
+  return [dir](const OmpDirectiveName &name) -> bool { return dir == name.v; };
+}
+
+static inline constexpr auto IsMemberOf(const DirectiveSet &dirs) {
+  return [&dirs](const OmpDirectiveName &name) -> bool {
+    return dirs.test(llvm::to_underlying(name.v));
+  };
+}
+
 TYPE_PARSER(sourced(construct<OmpDirectiveName>(OmpDirectiveNameParser{})))
 
 OmpDirectiveSpecification static makeFlushFromOldSyntax(Verbatim &&text,
@@ -1355,27 +1362,23 @@ struct LooselyStructuredBlockParser {
   }
 };
 
-TYPE_PARSER(sourced(construct<OmpNothingDirective>("NOTHING" >> ok)))
+TYPE_PARSER(construct<OmpErrorDirective>(
+    predicated(Parser<OmpDirectiveName>{},
+        IsDirective(llvm::omp::Directive::OMPD_error)) >=
+    Parser<OmpDirectiveSpecification>{}))
 
-TYPE_PARSER(sourced(construct<OpenMPUtilityConstruct>(
-    sourced(construct<OpenMPUtilityConstruct>(
-        sourced(Parser<OmpErrorDirective>{}))) ||
-    sourced(construct<OpenMPUtilityConstruct>(
-        sourced(Parser<OmpNothingDirective>{}))))))
+TYPE_PARSER(construct<OmpNothingDirective>(
+    predicated(Parser<OmpDirectiveName>{},
+        IsDirective(llvm::omp::Directive::OMPD_nothing)) >=
+    Parser<OmpDirectiveSpecification>{}))
+
+TYPE_PARSER( //
+    sourced(construct<OpenMPUtilityConstruct>(Parser<OmpErrorDirective>{})) ||
+    sourced(construct<OpenMPUtilityConstruct>(Parser<OmpNothingDirective>{})))
 
 TYPE_PARSER(sourced(construct<OmpMetadirectiveDirective>(
     verbatim("METADIRECTIVE"_tok), Parser<OmpClauseList>{})))
 
-static inline constexpr auto IsDirective(llvm::omp::Directive dir) {
-  return [dir](const OmpDirectiveName &name) -> bool { return dir == name.v; };
-}
-
-static inline constexpr auto IsMemberOf(const DirectiveSet &dirs) {
-  return [&dirs](const OmpDirectiveName &name) -> bool {
-    return dirs.test(llvm::to_underlying(name.v));
-  };
-}
-
 struct OmpBeginDirectiveParser {
   using resultType = OmpDirectiveSpecification;
 
diff --git a/flang/lib/Parser/unparse.cpp b/flang/lib/Parser/unparse.cpp
index 73bbbc04f46b1..9e455695b7b15 100644
--- a/flang/lib/Parser/unparse.cpp
+++ b/flang/lib/Parser/unparse.cpp
@@ -2610,13 +2610,18 @@ class UnparseVisitor {
     return false;
   }
   void Unparse(const OmpErrorDirective &x) {
-    Word("!$OMP ERROR ");
-    Walk(x.t);
+    BeginOpenMP();
+    Word("!$OMP ");
+    Walk(x.v);
     Put("\n");
+    EndOpenMP();
   }
   void Unparse(const OmpNothingDirective &x) {
-    Word("!$OMP NOTHING");
+    BeginOpenMP();
+    Word("!$OMP ");
+    Walk(x.v);
     Put("\n");
+    EndOpenMP();
   }
   void Unparse(const OmpSectionsDirective &x) {
     switch (x.v) {
diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index 4c7cd1734e0e7..266ef221ea7c9 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -608,14 +608,6 @@ template <typename Checker> struct DirectiveSpellingVisitor {
     checker_(GetDirName(x.t).source, Directive::OMPD_dispatch);
     return false;
   }
-  bool Pre(const parser::OmpErrorDirective &x) {
-    checker_(std::get<parser::Verbatim>(x.t).source, Directive::OMPD_error);
-    return false;
-  }
-  bool Pre(const parser::OmpNothingDirective &x) {
-    checker_(x.source, Directive::OMPD_nothing);
-    return false;
-  }
   bool Pre(const parser::OpenMPExecutableAllocate &x) {
     checker_(std::get<parser::Verbatim>(x.t).source, Directive::OMPD_allocate);
     return false;
@@ -1755,8 +1747,17 @@ void OmpStructureChecker::Leave(const parser::OpenMPDeclareTargetConstruct &x) {
 }
 
 void OmpStructureChecker::Enter(const parser::OmpErrorDirective &x) {
-  const auto &dir{std::get<parser::Verbatim>(x.t)};
-  PushContextAndClauseSets(dir.source, llvm::omp::Directive::OMPD_error);
+  const parser::OmpDirectiveName &dirName{x.v.DirName()};
+  PushContextAndClauseSets(dirName.source, dirName.v);
+}
+
+void OmpStructureChecker::Enter(const parser::OmpNothingDirective &x) {
+  const parser::OmpDirectiveName &dirName{x.v.DirName()};
+  PushContextAndClauseSets(dirName.source, dirName.v);
+}
+
+void OmpStructureChecker::Leave(const parser::OmpNothingDirective &x) {
+  dirContext_.pop_back();
 }
 
 void OmpStructureChecker::Enter(const parser::OpenMPDispatchConstruct &x) {
diff --git a/flang/lib/Semantics/check-omp-structure.h b/flang/lib/Semantics/check-omp-structure.h
index ce074f5f3f86e..81cabdfed0a39 100644
--- a/flang/lib/Semantics/check-omp-structure.h
+++ b/flang/lib/Semantics/check-omp-structure.h
@@ -120,6 +120,8 @@ class OmpStructureChecker
   void Leave(const parser::OpenMPDispatchConstruct &);
   void Enter(const parser::OmpErrorDirective &);
   void Leave(const parser::OmpErrorDirective &);
+  void Enter(const parser::OmpNothingDirective &);
+  void Leave(const parser::OmpNothingDirective &);
   void Enter(const parser::OpenMPExecutableAllocate &);
   void Leave(const parser::OpenMPExecutableAllocate &);
   void Enter(const parser::OpenMPAllocatorsConstruct &);

return [dir](const OmpDirectiveName &name) -> bool { return dir == name.v; };
}

static inline constexpr auto IsMemberOf(const DirectiveSet &dirs) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: There is no content change here, is it critical to move it or can we reduce the diff?

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in #159803.

Copy link
Contributor

@tblah tblah left a comment

Choose a reason for hiding this comment

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

LGTM, thanks

@kparzysz kparzysz merged commit c075fee into llvm:main Sep 22, 2025
9 checks passed
@kparzysz kparzysz deleted the users/kparzysz/ods-utility branch September 22, 2025 18:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

flang:openmp flang:parser flang:semantics flang Flang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants