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

[klausler]

Summary

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

[PeteSteinfeld]

Looks great!

flang/lib/Semantics/resolve-labels.cpp

public:
   // the construct as a whole must have a name that matches.
   template <typename FIRST, typename CONSTRUCT, typename STMT>
   void CheckOptionalName(const char *constructTag, const CONSTRUCT &a,
       const parser::Statement<STMT> &stmt) {
     if (const parser::CharBlock * name{GetStmtName(stmt)}) {
       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);
       }
     }
   }
 
   // C1414
   void Post(const parser::BlockData &blockData) {
     CheckOptionalName<parser::BlockDataStmt>("BLOCK DATA subprogram", blockData,
public:
   void Post(const parser::FunctionSubprogram &functionSubprogram) {
     CheckOptionalName<parser::FunctionStmt>("FUNCTION", functionSubprogram,
         std::get<parser::Statement<parser::EndFunctionStmt>>(
             functionSubprogram.t));
   }
 
   // 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{
+                .statement.v}) {
+      const auto &interfaceStmt{
+          std::get<parser::Statement<parser::InterfaceStmt>>(interfaceBlock.t)};
+      if (std::holds_alternative<parser::Abstract>(interfaceStmt.statement.u)) {
+            .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);
       }
     }
   }
 
   // C1402
   void Post(const parser::Module &module) {
     CheckOptionalName<parser::ModuleStmt>("MODULE", module,
         std::get<parser::Statement<parser::EndModuleStmt>>(module.t));
if (const parser::CharBlock *
               std::get<std::optional<parser::Statement<parser::ProgramStmt>>>(
                   mainProgram.t)}) {
         if (*endName != program->statement.v.source) {
           context_.Say(*endName, "END PROGRAM name mismatch"_err_en_US)
               .Attach(program->statement.v.source, "should be"_en_US);
         }
       } 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);
       }
     }
   }
 
   // C1413
   void Post(const parser::Submodule &submodule) {
     CheckOptionalName<parser::SubmoduleStmt>("SUBMODULE", submodule,
         std::get<parser::Statement<parser::EndSubmoduleStmt>>(submodule.t));
private:
   void CheckEndName(const char *constructTag, const CONSTRUCT &a) {
     const auto &constructStmt{std::get<parser::Statement<FIRST>>(a.t)};
     const auto &endStmt{std::get<parser::Statement<END>>(a.t)};
     const parser::CharBlock *endName{GetStmtName(endStmt)};
     if (const parser::CharBlock * constructName{GetStmtName(constructStmt)}) {
       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})
+                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);
     }
   }
 
   // C1106
   void CheckName(const parser::AssociateConstruct &associateConstruct) {
     CheckEndName<parser::AssociateStmt, parser::EndAssociateStmt>(
private:
   }
 
   // C1134, C1166
   void CheckLabelContext(
       const char *const stmtString, const parser::CharBlock &constructName) {
     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));
     }
   }
 
   // 6.2.5., paragraph 2
   void AddTargetLabelDefinition(parser::Label label,
       LabeledStmtClassificationSet labeledStmtClassificationSet,
       ProxyForScope scope, bool isExecutableConstructEndStmt = false) {
     CheckLabelInRange(label);
     const auto pair{programUnits_.back().targetStmts.emplace(label,
         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));
     }
   }
 
   void AddLabelReferenceFromDoStmt(parser::Label label) {
     CheckLabelInRange(label);
     programUnits_.back().doStmtSources.emplace_back(
         label, currentScope_, currentPosition_);
   }
private:
   void AddLabelReference(const std::list<parser::Label> &labels) {
     for (const parser::Label &label : labels) {
       AddLabelReference(label);
     }
   }
 
   std::vector<UnitAnalysis> programUnits_;
   SemanticsContext &context_;
-  parser::CharBlock currentPosition_{nullptr};
+  parser::CharBlock currentPosition_;
   ProxyForScope currentScope_;
   std::vector<std::string> constructNames_;
 };
 
 bool InInclusiveScope(const std::vector<ProxyForScope> &scopes,
     ProxyForScope tail, ProxyForScope head) {
   for (; tail != head; tail = scopes[tail]) {
     if (!HasScope(tail)) {
void CheckLabelDoConstraints(const SourceStmtList &dos,
   IndexList loopBodies;
   for (const auto &stmt : dos) {
     const auto &label{stmt.parserLabel};
     const auto &scope{stmt.proxyForScope};
     const auto &position{stmt.parserCharBlock};
     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)});
+          SayLabel(label));
 
     } else if ((InInclusiveScope(scopes, scope, doTarget.proxyForScope) &&
                    doTarget.labeledStmtClassificationSet.test(
                        TargetStatementEnum::CompatibleDo)) ||
         (doTarget.isExecutableConstructEndStmt &&
             ParentScope(scopes, doTarget.proxyForScope) == scope)) {
       if (context.warnOnNonstandardUsage() ||
           context.ShouldWarn(
               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);
     }
   }
 
   CheckBranchesIntoDoBody(branches, labels, loopBodies, context);
   CheckDoNesting(loopBodies, context);
 }
 
 // 6.2.5
 void CheckScopeConstraints(const SourceStmtList &stmts,
     const TargetStmtMap &labels, const std::vector<ProxyForScope> &scopes,
     SemanticsContext &context) {
   for (const auto &stmt : stmts) {
     const auto &label{stmt.parserLabel};
     const auto &scope{stmt.proxyForScope};
     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.
       if (target.labeledStmtClassificationSet.test(
               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));
     }
   }
 }
 
 void CheckBranchTargetConstraints(const SourceStmtList &stmts,
     const TargetStmtMap &labels, SemanticsContext &context) {
   for (const auto &stmt : stmts) {
     const auto &label{stmt.parserLabel};
     auto branchTarget{GetLabel(labels, label)};
     if (HasScope(branchTarget.proxyForScope)) {
       if (!branchTarget.labeledStmtClassificationSet.test(
               TargetStatementEnum::Branch) &&
           !branchTarget.labeledStmtClassificationSet.test(
               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));
       }
     }
   }
 }
 
 void CheckBranchConstraints(const SourceStmtList &branches,
     const TargetStmtMap &labels, const std::vector<ProxyForScope> &scopes,
     SemanticsContext &context) {
   CheckScopeConstraints(branches, labels, scopes, context);
   CheckBranchTargetConstraints(branches, labels, context);
 }
 
 void CheckDataXferTargetConstraints(const SourceStmtList &stmts,
     const TargetStmtMap &labels, SemanticsContext &context) {
   for (const auto &stmt : stmts) {
     const auto &label{stmt.parserLabel};
     auto ioTarget{GetLabel(labels, label)};
     if (HasScope(ioTarget.proxyForScope)) {
       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));
       }
     }
   }
 }
 
 void CheckDataTransferConstraints(const SourceStmtList &dataTransfers,
     const TargetStmtMap &labels, const std::vector<ProxyForScope> &scopes,
     SemanticsContext &context) {

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

 std::optional<std::int64_t> EvaluateInt64(
     SemanticsContext &context, const T &expr) {
   return evaluate::ToInt64(EvaluateIntExpr(context, expr));
 }
 
 // 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(); }
   // Set the GenericKind in this symbol and resolve the corresponding
   // name if there is one
   void Resolve(Symbol *) const;
   friend llvm::raw_ostream &operator<<(
       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
 ArraySpec AnalyzeArraySpec(SemanticsContext &, const parser::ArraySpec &);
 ArraySpec AnalyzeArraySpec(
     SemanticsContext &, const parser::ComponentArraySpec &);
 ArraySpec AnalyzeDeferredShapeSpecList(
     SemanticsContext &, const parser::DeferredShapeSpecList &);

flang/lib/Semantics/resolve-names.cpp

void ScopeHandler::MakeExternal(Symbol &symbol) {
   }
 }
 
 // ModuleVisitor implementation
 
 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);
                  },
                  [&](const parser::Name &name) {
                    AddUseOnly(name.source);
                    Resolve(name, AddUse(name.source, name.source).use);
                  },
                  [&](const parser::Rename &rename) { Walk(rename); },

flang/test/Semantics/label11.f90

 end program t14
 
 submodule (mod) t15
 !ERROR: SUBMODULE name mismatch
 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
 
   abstract interface
     subroutine subrFront()
     !ERROR: SUBROUTINE name mismatch
     end subroutine subrBack