[flang] Revamp C1502 checking of END INTERFACE [generic-spec]

Validation of the optional generic-spec on an END INTERFACE statement
was missing many possible error cases; reimplement it.

Differential Revision: https://reviews.llvm.org/D109910

flang/lib/Semantics/resolve-labels.cpp

@@ -346,17 +346,11 @@ public:
       const auto &firstStmt{std::get<parser::Statement<FIRST>>(a.t)};
       if (const parser::CharBlock * firstName{GetStmtName(firstStmt)}) {
         if (*firstName != *name) {
-          context_
-              .Say(*name,
-                  parser::MessageFormattedText{
-                      "%s name mismatch"_err_en_US, constructTag})
+          context_.Say(*name, "%s name mismatch"_err_en_US, constructTag)
               .Attach(*firstName, "should be"_en_US);
         }
       } else {
-        context_
-            .Say(*name,
-                parser::MessageFormattedText{
-                    "%s name not allowed"_err_en_US, constructTag})
+        context_.Say(*name, "%s name not allowed"_err_en_US, constructTag)
             .Attach(firstStmt.source, "in unnamed %s"_en_US, constructTag);
       }
     }
@@ -383,32 +377,51 @@ public:
 
   // C1502
   void Post(const parser::InterfaceBlock &interfaceBlock) {
-    auto &interfaceStmt{
-        std::get<parser::Statement<parser::InterfaceStmt>>(interfaceBlock.t)};
-    if (const auto *optionalGenericSpecPointer{
-            std::get_if<std::optional<parser::GenericSpec>>(
-                &interfaceStmt.statement.u)}) {
-      if (*optionalGenericSpecPointer) {
-        if (const auto *namePointer{
-                std::get_if<parser::Name>(&(*optionalGenericSpecPointer)->u)}) {
-          auto &optionalGenericSpec{
-              std::get<parser::Statement<parser::EndInterfaceStmt>>(
-                  interfaceBlock.t)
-                  .statement.v};
-          if (optionalGenericSpec) {
-            if (const auto *otherPointer{
-                    std::get_if<parser::Name>(&optionalGenericSpec->u)}) {
-              if (namePointer->source != otherPointer->source) {
-                context_
-                    .Say(currentPosition_,
-                        parser::MessageFormattedText{
-                            "INTERFACE generic-name (%s) mismatch"_err_en_US,
-                            namePointer->source})
-                    .Attach(interfaceStmt.source, "mismatched INTERFACE"_en_US);
-              }
-            }
+    if (const auto &endGenericSpec{
+            std::get<parser::Statement<parser::EndInterfaceStmt>>(
+                interfaceBlock.t)
+                .statement.v}) {
+      const auto &interfaceStmt{
+          std::get<parser::Statement<parser::InterfaceStmt>>(interfaceBlock.t)};
+      if (std::holds_alternative<parser::Abstract>(interfaceStmt.statement.u)) {
+        context_
+            .Say(endGenericSpec->source,
+                "END INTERFACE generic name (%s) may not appear for ABSTRACT INTERFACE"_err_en_US,
+                endGenericSpec->source)
+            .Attach(
+                interfaceStmt.source, "corresponding ABSTRACT INTERFACE"_en_US);
+      } else if (const auto &genericSpec{
+                     std::get<std::optional<parser::GenericSpec>>(
+                         interfaceStmt.statement.u)}) {
+        bool ok{genericSpec->source == endGenericSpec->source};
+        if (!ok) {
+          // Accept variant spellings of .LT. &c.
+          const auto *endOp{
+              std::get_if<parser::DefinedOperator>(&endGenericSpec->u)};
+          const auto *op{std::get_if<parser::DefinedOperator>(&genericSpec->u)};
+          if (endOp && op) {
+            const auto *endIntrin{
+                std::get_if<parser::DefinedOperator::IntrinsicOperator>(
+                    &endOp->u)};
+            const auto *intrin{
+                std::get_if<parser::DefinedOperator::IntrinsicOperator>(
+                    &op->u)};
+            ok = endIntrin && intrin && *endIntrin == *intrin;
           }
         }
+        if (!ok) {
+          context_
+              .Say(endGenericSpec->source,
+                  "END INTERFACE generic name (%s) does not match generic INTERFACE (%s)"_err_en_US,
+                  endGenericSpec->source, genericSpec->source)
+              .Attach(genericSpec->source, "corresponding INTERFACE"_en_US);
+        }
+      } else {
+        context_
+            .Say(endGenericSpec->source,
+                "END INTERFACE generic name (%s) may not appear for non-generic INTERFACE"_err_en_US,
+                endGenericSpec->source)
+            .Attach(interfaceStmt.source, "corresponding INTERFACE"_en_US);
       }
     }
   }
@@ -441,8 +454,7 @@ public:
         }
       } else {
         context_.Say(*endName,
-            parser::MessageFormattedText{
-                "END PROGRAM has name without PROGRAM statement"_err_en_US});
+            "END PROGRAM has name without PROGRAM statement"_err_en_US);
       }
     }
   }
@@ -640,24 +652,20 @@ private:
       if (endName) {
         if (*constructName != *endName) {
           context_
-              .Say(*endName,
-                  parser::MessageFormattedText{
-                      "%s construct name mismatch"_err_en_US, constructTag})
+              .Say(*endName, "%s construct name mismatch"_err_en_US,
+                  constructTag)
               .Attach(*constructName, "should be"_en_US);
         }
       } else {
         context_
             .Say(endStmt.source,
-                parser::MessageFormattedText{
-                    "%s construct name required but missing"_err_en_US,
-                    constructTag})
+                "%s construct name required but missing"_err_en_US,
+                constructTag)
             .Attach(*constructName, "should be"_en_US);
       }
     } else if (endName) {
       context_
-          .Say(*endName,
-              parser::MessageFormattedText{
-                  "%s construct name unexpected"_err_en_US, constructTag})
+          .Say(*endName, "%s construct name unexpected"_err_en_US, constructTag)
           .Attach(
               constructStmt.source, "unnamed %s statement"_en_US, constructTag);
     }
@@ -737,18 +745,16 @@ private:
     const auto iter{std::find(constructNames_.crbegin(),
         constructNames_.crend(), constructName.ToString())};
     if (iter == constructNames_.crend()) {
-      context_.Say(constructName,
-          parser::MessageFormattedText{
-              "%s construct-name is not in scope"_err_en_US, stmtString});
+      context_.Say(constructName, "%s construct-name is not in scope"_err_en_US,
+          stmtString);
     }
   }
 
   // 6.2.5, paragraph 2
   void CheckLabelInRange(parser::Label label) {
     if (label < 1 || label > 99999) {
-      context_.Say(currentPosition_,
-          parser::MessageFormattedText{
-              "Label '%u' is out of range"_err_en_US, SayLabel(label)});
+      context_.Say(currentPosition_, "Label '%u' is out of range"_err_en_US,
+          SayLabel(label));
     }
   }
 
@@ -761,9 +767,8 @@ private:
         LabeledStatementInfoTuplePOD{scope, currentPosition_,
             labeledStmtClassificationSet, isExecutableConstructEndStmt})};
     if (!pair.second) {
-      context_.Say(currentPosition_,
-          parser::MessageFormattedText{
-              "Label '%u' is not distinct"_err_en_US, SayLabel(label)});
+      context_.Say(currentPosition_, "Label '%u' is not distinct"_err_en_US,
+          SayLabel(label));
     }
   }
 
@@ -799,7 +804,7 @@ private:
 
   std::vector<UnitAnalysis> programUnits_;
   SemanticsContext &context_;
-  parser::CharBlock currentPosition_{nullptr};
+  parser::CharBlock currentPosition_;
   ProxyForScope currentScope_;
   std::vector<std::string> constructNames_;
 };
@@ -904,15 +909,13 @@ void CheckLabelDoConstraints(const SourceStmtList &dos,
     auto doTarget{GetLabel(labels, label)};
     if (!HasScope(doTarget.proxyForScope)) {
       // C1133
-      context.Say(position,
-          parser::MessageFormattedText{
-              "Label '%u' cannot be found"_err_en_US, SayLabel(label)});
+      context.Say(
+          position, "Label '%u' cannot be found"_err_en_US, SayLabel(label));
     } else if (doTarget.parserCharBlock.begin() < position.begin()) {
       // R1119
       context.Say(position,
-          parser::MessageFormattedText{
-              "Label '%u' doesn't lexically follow DO stmt"_err_en_US,
-              SayLabel(label)});
+          "Label '%u' doesn't lexically follow DO stmt"_err_en_US,
+          SayLabel(label));
 
     } else if ((InInclusiveScope(scopes, scope, doTarget.proxyForScope) &&
                    doTarget.labeledStmtClassificationSet.test(
@@ -924,20 +927,17 @@ void CheckLabelDoConstraints(const SourceStmtList &dos,
               common::LanguageFeature::OldLabelDoEndStatements)) {
         context
             .Say(position,
-                parser::MessageFormattedText{
-                    "A DO loop should terminate with an END DO or CONTINUE"_en_US})
+                "A DO loop should terminate with an END DO or CONTINUE"_en_US)
             .Attach(doTarget.parserCharBlock,
                 "DO loop currently ends at statement:"_en_US);
       }
     } else if (!InInclusiveScope(scopes, scope, doTarget.proxyForScope)) {
-      context.Say(position,
-          parser::MessageFormattedText{
-              "Label '%u' is not in DO loop scope"_err_en_US, SayLabel(label)});
+      context.Say(position, "Label '%u' is not in DO loop scope"_err_en_US,
+          SayLabel(label));
     } else if (!doTarget.labeledStmtClassificationSet.test(
                    TargetStatementEnum::Do)) {
       context.Say(doTarget.parserCharBlock,
-          parser::MessageFormattedText{
-              "A DO loop should terminate with an END DO or CONTINUE"_err_en_US});
+          "A DO loop should terminate with an END DO or CONTINUE"_err_en_US);
     } else {
       loopBodies.emplace_back(SkipLabel(position), doTarget.parserCharBlock);
     }
@@ -957,9 +957,8 @@ void CheckScopeConstraints(const SourceStmtList &stmts,
     const auto &position{stmt.parserCharBlock};
     auto target{GetLabel(labels, label)};
     if (!HasScope(target.proxyForScope)) {
-      context.Say(position,
-          parser::MessageFormattedText{
-              "Label '%u' was not found"_err_en_US, SayLabel(label)});
+      context.Say(
+          position, "Label '%u' was not found"_err_en_US, SayLabel(label));
     } else if (!InInclusiveScope(scopes, scope, target.proxyForScope)) {
       // Clause 11.1.2.1 prohibits transfer of control to the interior of a
       // block from outside the block, but this does not apply to formats.
@@ -967,9 +966,8 @@ void CheckScopeConstraints(const SourceStmtList &stmts,
               TargetStatementEnum::Format)) {
         continue;
       }
-      context.Say(position,
-          parser::MessageFormattedText{
-              "Label '%u' is not in scope"_en_US, SayLabel(label)});
+      context.Say(
+          position, "Label '%u' is not in scope"_en_US, SayLabel(label));
     }
   }
 }
@@ -986,21 +984,16 @@ void CheckBranchTargetConstraints(const SourceStmtList &stmts,
               TargetStatementEnum::CompatibleBranch)) { // error
         context
             .Say(branchTarget.parserCharBlock,
-                parser::MessageFormattedText{
-                    "Label '%u' is not a branch target"_err_en_US,
-                    SayLabel(label)})
-            .Attach(stmt.parserCharBlock,
-                parser::MessageFormattedText{
-                    "Control flow use of '%u'"_en_US, SayLabel(label)});
+                "Label '%u' is not a branch target"_err_en_US, SayLabel(label))
+            .Attach(stmt.parserCharBlock, "Control flow use of '%u'"_en_US,
+                SayLabel(label));
       } else if (!branchTarget.labeledStmtClassificationSet.test(
                      TargetStatementEnum::Branch)) { // warning
         context
             .Say(branchTarget.parserCharBlock,
-                parser::MessageFormattedText{
-                    "Label '%u' is not a branch target"_en_US, SayLabel(label)})
-            .Attach(stmt.parserCharBlock,
-                parser::MessageFormattedText{
-                    "Control flow use of '%u'"_en_US, SayLabel(label)});
+                "Label '%u' is not a branch target"_en_US, SayLabel(label))
+            .Attach(stmt.parserCharBlock, "Control flow use of '%u'"_en_US,
+                SayLabel(label));
       }
     }
   }
@@ -1022,12 +1015,10 @@ void CheckDataXferTargetConstraints(const SourceStmtList &stmts,
       if (!ioTarget.labeledStmtClassificationSet.test(
               TargetStatementEnum::Format)) {
         context
-            .Say(ioTarget.parserCharBlock,
-                parser::MessageFormattedText{
-                    "'%u' not a FORMAT"_err_en_US, SayLabel(label)})
-            .Attach(stmt.parserCharBlock,
-                parser::MessageFormattedText{
-                    "data transfer use of '%u'"_en_US, SayLabel(label)});
+            .Say(ioTarget.parserCharBlock, "'%u' not a FORMAT"_err_en_US,
+                SayLabel(label))
+            .Attach(stmt.parserCharBlock, "data transfer use of '%u'"_en_US,
+                SayLabel(label));
       }
     }
   }

flang/lib/Semantics/resolve-names-utils.h

@@ -76,8 +76,8 @@ std::optional<std::int64_t> EvaluateInt64(
 // Analyze a generic-spec and generate a symbol name and GenericKind for it.
 class GenericSpecInfo {
 public:
-  GenericSpecInfo(const parser::DefinedOpName &x) { Analyze(x); }
-  GenericSpecInfo(const parser::GenericSpec &x) { Analyze(x); }
+  explicit GenericSpecInfo(const parser::DefinedOpName &x) { Analyze(x); }
+  explicit GenericSpecInfo(const parser::GenericSpec &x) { Analyze(x); }
 
   GenericKind kind() const { return kind_; }
   const SourceName &symbolName() const { return symbolName_.value(); }
@@ -88,12 +88,12 @@ public:
       llvm::raw_ostream &, const GenericSpecInfo &);
 
 private:
+  void Analyze(const parser::DefinedOpName &);
+  void Analyze(const parser::GenericSpec &);
+
   GenericKind kind_;
   const parser::Name *parseName_{nullptr};
   std::optional<SourceName> symbolName_;
-
-  void Analyze(const parser::DefinedOpName &);
-  void Analyze(const parser::GenericSpec &);
 };
 
 // Analyze a parser::ArraySpec or parser::CoarraySpec

flang/lib/Semantics/resolve-names.cpp

@@ -2423,7 +2423,7 @@ void ScopeHandler::MakeExternal(Symbol &symbol) {
 bool ModuleVisitor::Pre(const parser::Only &x) {
   std::visit(common::visitors{
                  [&](const Indirection<parser::GenericSpec> &generic) {
-                   const GenericSpecInfo &genericSpecInfo{generic.value()};
+                   GenericSpecInfo genericSpecInfo{generic.value()};
                    AddUseOnly(genericSpecInfo.symbolName());
                    AddUse(genericSpecInfo);
                  },

flang/test/Semantics/label11.f90

@@ -41,8 +41,22 @@ end submodule t16
 
 module t5
   interface t7
-  !ERROR: INTERFACE generic-name (t7) mismatch
+  !ERROR: END INTERFACE generic name (t8) does not match generic INTERFACE (t7)
   end interface t8
+  abstract interface
+  !ERROR: END INTERFACE generic name (t19) may not appear for ABSTRACT INTERFACE
+  end interface t19
+  interface
+  !ERROR: END INTERFACE generic name (t20) may not appear for non-generic INTERFACE
+  end interface t20
+  interface
+  !ERROR: END INTERFACE generic name (assignment(=)) may not appear for non-generic INTERFACE
+  end interface assignment(=)
+  interface operator(<)
+  end interface operator(.LT.) ! not an error
+  interface operator(.EQ.)
+  end interface operator(==) ! not an error
+
   type t17
   !ERROR: derived type definition name mismatch
   end type t18