[flang][OpenMP] Use OmpDirectiveSpecification in DECLARE_VARIANT

flang/include/flang/Parser/dump-parse-tree.h

@@ -522,6 +522,7 @@ class ParseTreeDumper {
   NODE(parser, OmpAtomicDefaultMemOrderClause)
   NODE(parser, OmpAutomapModifier)
   NODE_ENUM(OmpAutomapModifier, Value)
+  NODE(parser, OmpBaseVariantNames)
   NODE(parser, OmpBeginDirective)
   NODE(parser, OmpBeginLoopDirective)
   NODE(parser, OmpBeginSectionsDirective)

flang/include/flang/Parser/openmp-utils.h

@@ -38,7 +38,6 @@ struct ConstructId {
     static constexpr llvm::omp::Directive id{Id}; \
   }
 
-MAKE_CONSTR_ID(OmpDeclareVariantDirective, D::OMPD_declare_variant);
 MAKE_CONSTR_ID(OpenMPDeclarativeAllocate, D::OMPD_allocate);
 MAKE_CONSTR_ID(OpenMPDeclarativeAssumes, D::OMPD_assumes);
 MAKE_CONSTR_ID(OpenMPDeclareReductionConstruct, D::OMPD_declare_reduction);
@@ -92,8 +91,7 @@ struct DirectiveNameScope {
     } else if constexpr (TupleTrait<T>) {
       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, OpenMPDeclarativeAllocate> ||
+      } else if constexpr (std::is_same_v<T, OpenMPDeclarativeAllocate> ||
           std::is_same_v<T, OpenMPDeclarativeAssumes> ||
           std::is_same_v<T, OpenMPDeclareReductionConstruct> ||
           std::is_same_v<T, OpenMPDeclareSimdConstruct> ||

flang/include/flang/Parser/parse-tree.h

@@ -3555,6 +3555,18 @@ struct OmpLocator {
 
 WRAPPER_CLASS(OmpLocatorList, std::list<OmpLocator>);
 
+// Ref: [4.5:58-60], [5.0:58-60], [5.1:63-68], [5.2:197-198], [6.0:334-336]
+//
+// Argument to DECLARE VARIANT with the base-name present. (When only
+// variant-name is present, it is a simple OmpObject).
+//
+// base-name-variant-name ->                        // since 4.5
+//    base-name : variant-name
+struct OmpBaseVariantNames {
+  TUPLE_CLASS_BOILERPLATE(OmpBaseVariantNames);
+  std::tuple<OmpObject, OmpObject> t;
+};
+
 // Ref: [5.0:326:10-16], [5.1:359:5-11], [5.2:163:2-7], [6.0:293:16-21]
 //
 // mapper-specifier ->
@@ -3584,6 +3596,7 @@ struct OmpArgument {
   CharBlock source;
   UNION_CLASS_BOILERPLATE(OmpArgument);
   std::variant<OmpLocator, // {variable, extended, locator}-list-item
+      OmpBaseVariantNames, // base-name:variant-name
       OmpMapperSpecifier, OmpReductionSpecifier>
       u;
 };
@@ -4920,10 +4933,14 @@ struct OpenMPSectionsConstruct {
       t;
 };
 
+// Ref: [4.5:58-60], [5.0:58-60], [5.1:63-68], [5.2:197-198], [6.0:334-336]
+//
+// declare-variant-directive ->
+//    DECLARE_VARIANT([base-name:]variant-name)     // since 4.5
 struct OmpDeclareVariantDirective {
-  TUPLE_CLASS_BOILERPLATE(OmpDeclareVariantDirective);
+  WRAPPER_CLASS_BOILERPLATE(
+      OmpDeclareVariantDirective, OmpDirectiveSpecification);
   CharBlock source;
-  std::tuple<Verbatim, std::optional<Name>, Name, OmpClauseList> t;
 };
 
 // 2.10.6 declare-target -> DECLARE TARGET (extended-list) |

flang/lib/Parser/openmp-parsers.cpp

@@ -315,15 +315,56 @@ TYPE_PARSER( //
     construct<OmpLocator>(Parser<OmpObject>{}) ||
     construct<OmpLocator>(Parser<FunctionReference>{}))
 
-TYPE_PARSER(sourced( //
-    construct<OmpArgument>(Parser<OmpMapperSpecifier>{}) ||
-    construct<OmpArgument>(Parser<OmpReductionSpecifier>{}) ||
-    construct<OmpArgument>(Parser<OmpLocator>{})))
+TYPE_PARSER(construct<OmpBaseVariantNames>(
+    Parser<OmpObject>{} / ":", Parser<OmpObject>{}))
+
+// Make the parsing of OmpArgument directive-sensitive. The issue is that
+// name1:name2 can match either OmpBaseVariantNames or OmpReductionSpecifier.
+// In the former case, "name2" is a name of a function, in the latter, of a
+// type. To resolve the conflict we need information provided by name
+// resolution, but by that time we can't modify the AST anymore, and the
+// name resolution may have implicitly declared a symbol, or issued a message.
+template <llvm::omp::Directive Id = llvm::omp::Directive::OMPD_unknown>
+struct OmpArgumentParser {
+  using resultType = OmpArgument;
+
+  std::optional<resultType> Parse(ParseState &state) const {
+    constexpr auto parser{sourced(first( //
+        construct<OmpArgument>(Parser<OmpMapperSpecifier>{}),
+        // By default, prefer OmpReductionSpecifier over OmpBaseVariantNames.
+        construct<OmpArgument>(Parser<OmpReductionSpecifier>{}),
+        construct<OmpArgument>(Parser<OmpLocator>{})))};
+    return parser.Parse(state);
+  }
+};
+
+template <>
+struct OmpArgumentParser<llvm::omp::Directive::OMPD_declare_variant> {
+  using resultType = OmpArgument;
+
+  std::optional<resultType> Parse(ParseState &state) const {
+    constexpr auto parser{sourced(first( //
+        construct<OmpArgument>(Parser<OmpMapperSpecifier>{}),
+        // In DECLARE_VARIANT parse OmpBaseVariantNames instead of
+        // OmpReductionSpecifier.
+        construct<OmpArgument>(Parser<OmpBaseVariantNames>{}),
+        construct<OmpArgument>(Parser<OmpLocator>{})))};
+    return parser.Parse(state);
+  }
+};
 
 TYPE_PARSER(construct<OmpLocatorList>(nonemptyList(Parser<OmpLocator>{})))
 
-TYPE_PARSER(sourced( //
-    construct<OmpArgumentList>(nonemptyList(Parser<OmpArgument>{}))))
+template <llvm::omp::Directive Id = llvm::omp::Directive::OMPD_unknown>
+struct OmpArgumentListParser {
+  using resultType = OmpArgumentList;
+
+  std::optional<resultType> Parse(ParseState &state) const {
+    return sourced(
+        construct<OmpArgumentList>(nonemptyList(OmpArgumentParser<Id>{})))
+        .Parse(state);
+  }
+};
 
 TYPE_PARSER( //
     construct<OmpTypeSpecifier>(Parser<DeclarationTypeSpec>{}) ||
@@ -1312,12 +1353,23 @@ TYPE_PARSER(
         applyFunction<OmpDirectiveSpecification>(makeFlushFromOldSyntax,
             verbatim("FLUSH"_tok) / !lookAhead("("_tok),
             maybe(Parser<OmpClauseList>{}),
-            maybe(parenthesized(Parser<OmpArgumentList>{})),
+            maybe(parenthesized(
+                OmpArgumentListParser<llvm::omp::Directive::OMPD_flush>{})),
             pure(OmpDirectiveSpecification::Flags::DeprecatedSyntax)))) ||
+    // Parse DECLARE_VARIANT individually, because the "[base:]variant"
+    // argument will conflict with DECLARE_REDUCTION's "ident:types...".
+    predicated(Parser<OmpDirectiveName>{},
+        IsDirective(llvm::omp::Directive::OMPD_declare_variant)) >=
+        sourced(construct<OmpDirectiveSpecification>(
+            sourced(OmpDirectiveNameParser{}),
+            maybe(parenthesized(OmpArgumentListParser<
+                llvm::omp::Directive::OMPD_declare_variant>{})),
+            maybe(Parser<OmpClauseList>{}),
+            pure(OmpDirectiveSpecification::Flags::None))) ||
     // Parse the standard syntax: directive [(arguments)] [clauses]
     sourced(construct<OmpDirectiveSpecification>( //
         sourced(OmpDirectiveNameParser{}),
-        maybe(parenthesized(Parser<OmpArgumentList>{})),
+        maybe(parenthesized(OmpArgumentListParser<>{})),
         maybe(Parser<OmpClauseList>{}),
         pure(OmpDirectiveSpecification::Flags::None))))
 
@@ -1711,8 +1763,9 @@ TYPE_PARSER(construct<OmpInitializerClause>(
 
 // OpenMP 5.2: 7.5.4 Declare Variant directive
 TYPE_PARSER(sourced(construct<OmpDeclareVariantDirective>(
-    verbatim("DECLARE VARIANT"_tok) || verbatim("DECLARE_VARIANT"_tok),
-    "(" >> maybe(name / ":"), name / ")", Parser<OmpClauseList>{})))
+    predicated(Parser<OmpDirectiveName>{},
+        IsDirective(llvm::omp::Directive::OMPD_declare_variant)) >=
+    Parser<OmpDirectiveSpecification>{})))
 
 // 2.16 Declare Reduction Construct
 TYPE_PARSER(sourced(construct<OpenMPDeclareReductionConstruct>(

flang/lib/Parser/unparse.cpp

@@ -2089,6 +2089,11 @@ class UnparseVisitor {
   // OpenMP Clauses & Directives
   void Unparse(const OmpArgumentList &x) { Walk(x.v, ", "); }
 
+  void Unparse(const OmpBaseVariantNames &x) {
+    Walk(std::get<0>(x.t)); // OmpObject
+    Put(":");
+    Walk(std::get<1>(x.t)); // OmpObject
+  }
   void Unparse(const OmpTypeNameList &x) { //
     Walk(x.v, ",");
   }
@@ -2534,12 +2539,8 @@ class UnparseVisitor {
   }
   void Unparse(const OmpDeclareVariantDirective &x) {
     BeginOpenMP();
-    Word("!$OMP DECLARE VARIANT ");
-    Put("(");
-    Walk(std::get<std::optional<Name>>(x.t), ":");
-    Walk(std::get<Name>(x.t));
-    Put(")");
-    Walk(std::get<OmpClauseList>(x.t));
+    Word("!$OMP ");
+    Walk(x.v);
     Put("\n");
     EndOpenMP();
   }

flang/lib/Semantics/check-omp-structure.cpp

@@ -634,11 +634,6 @@ template <typename Checker> struct DirectiveSpellingVisitor {
         std::get<parser::Verbatim>(x.t).source, Directive::OMPD_declare_target);
     return false;
   }
-  bool Pre(const parser::OmpDeclareVariantDirective &x) {
-    checker_(std::get<parser::Verbatim>(x.t).source,
-        Directive::OMPD_declare_variant);
-    return false;
-  }
   bool Pre(const parser::OpenMPGroupprivate &x) {
     checker_(x.v.DirName().source, Directive::OMPD_groupprivate);
     return false;
@@ -1370,9 +1365,50 @@ void OmpStructureChecker::Leave(const parser::OpenMPDeclareSimdConstruct &) {
 }
 
 void OmpStructureChecker::Enter(const parser::OmpDeclareVariantDirective &x) {
-  const auto &dir{std::get<parser::Verbatim>(x.t)};
-  PushContextAndClauseSets(
-      dir.source, llvm::omp::Directive::OMPD_declare_variant);
+  const parser::OmpDirectiveName &dirName{x.v.DirName()};
+  PushContextAndClauseSets(dirName.source, dirName.v);
+
+  const parser::OmpArgumentList &args{x.v.Arguments()};
+  if (args.v.size() != 1) {
+    context_.Say(args.source,
+        "DECLARE_VARIANT directive should have a single argument"_err_en_US);
+    return;
+  }
+
+  auto InvalidArgument{[&](parser::CharBlock source) {
+    context_.Say(source,
+        "The argument to the DECLARE_VARIANT directive should be [base-name:]variant-name"_err_en_US);
+  }};
+
+  auto CheckSymbol{[&](const Symbol *sym, parser::CharBlock source) {
+    if (sym) {
+      if (!IsProcedure(*sym) && !IsFunction(*sym)) {
+        context_.Say(source,
+            "The name '%s' should refer to a procedure"_err_en_US, sym->name());
+      }
+      if (sym->test(Symbol::Flag::Implicit)) {
+        context_.Say(source,
+            "The name '%s' has been implicitly declared"_err_en_US,
+            sym->name());
+      }
+    } else {
+      InvalidArgument(source);
+    }
+  }};
+
+  const parser::OmpArgument &arg{args.v.front()};
+  common::visit( //
+      common::visitors{
+          [&](const parser::OmpBaseVariantNames &y) {
+            CheckSymbol(GetObjectSymbol(std::get<0>(y.t)), arg.source);
+            CheckSymbol(GetObjectSymbol(std::get<1>(y.t)), arg.source);
+          },
+          [&](const parser::OmpLocator &y) {
+            CheckSymbol(GetArgumentSymbol(arg), arg.source);
+          },
+          [&](auto &&y) { InvalidArgument(arg.source); },
+      },
+      arg.u);
 }
 
 void OmpStructureChecker::Leave(const parser::OmpDeclareVariantDirective &) {

flang/lib/Semantics/resolve-names.cpp

@@ -1540,20 +1540,6 @@ class OmpVisitor : public virtual DeclarationVisitor {
 
   bool Pre(const parser::OmpDeclareVariantDirective &x) {
     AddOmpSourceRange(x.source);
-    auto FindSymbolOrError = [&](const parser::Name &procName) {
-      auto *symbol{FindSymbol(NonDerivedTypeScope(), procName)};
-      if (!symbol) {
-        context().Say(procName.source,
-            "Implicit subroutine declaration '%s' in !$OMP DECLARE VARIANT"_err_en_US,
-            procName.source);
-      }
-    };
-    auto &baseProcName = std::get<std::optional<parser::Name>>(x.t);
-    if (baseProcName) {
-      FindSymbolOrError(*baseProcName);
-    }
-    auto &varProcName = std::get<parser::Name>(x.t);
-    FindSymbolOrError(varProcName);
     return true;
   }
 
@@ -1687,16 +1673,19 @@ class OmpVisitor : public virtual DeclarationVisitor {
       PopScope();
     }
   }
+
+  // These objects are handled explicitly, and the AST traversal should not
+  // reach a point where it calls the Pre functions for them.
   bool Pre(const parser::OmpMapperSpecifier &x) {
-    // OmpMapperSpecifier is handled explicitly, and the AST traversal
-    // should not reach a point where it calls this function.
     llvm_unreachable("This function should not be reached by AST traversal");
   }
   bool Pre(const parser::OmpReductionSpecifier &x) {
-    // OmpReductionSpecifier is handled explicitly, and the AST traversal
-    // should not reach a point where it calls this function.
     llvm_unreachable("This function should not be reached by AST traversal");
   }
+  bool Pre(const parser::OmpBaseVariantNames &x) {
+    llvm_unreachable("This function should not be reached by AST traversal");
+  }
+
   bool Pre(const parser::OmpDirectiveSpecification &x);
   void Post(const parser::OmpDirectiveSpecification &) {
     messageHandler().set_currStmtSource(std::nullopt);
@@ -1998,6 +1987,10 @@ bool OmpVisitor::Pre(const parser::OmpDirectiveSpecification &x) {
               ProcessReductionSpecifier(spec, clauses);
               visitClauses = false;
             },
+            [&](const parser::OmpBaseVariantNames &names) {
+              Walk(std::get<0>(names.t));
+              Walk(std::get<1>(names.t));
+            },
             [&](const parser::OmpLocator &locator) {
               // Manually resolve names in CRITICAL directives. This is because
               // these names do not denote Fortran objects, and the CRITICAL