[flang] Fix bogus message on interface procedure argument names

[PeteSteinfeld]

Summary

We were keeping the state of parsed equivalence sets in the class
DeclarationVisitor. A problem happened when analyzing the the specification
part of a declaration that contained an EQUIVALENCE statement followed by an
interface block. The same DeclarationVisitor object that was created for the
outer declaration was being used to analyze the specification part
of a procedure body in the interface block. When analyzing the specification
part of the procedure in the interface block, the names in the outer
declaration's EQUIVALENCE statement were erroneously compared with the names in
the arguments of the interface procedure. This resulted in a bogus error
message.

I fixed this by checking to see if we were in an interface block before emitting an error message for EQUIVALENCE statement processing. I also added a test that will produce an erroneous error message without this change.

[klausler]

I don't think that moving a temporary data structure from declaration processing into a more permanent object like Scope to accomplish scoping is the right way to fix this problem, unfortunately.

The specification-part of an interface block exists only to define the dummy arguments and (if there is one) the function result, so declarations of other local variables are moot in an interface outside of specification inquiries like SIZE(). Since local variables are the only objects that could meaningfully appear in an EQUIVALENCE statement in the specification part of an interface -- dummies can't be EQUIVALENCEd and there's nothing to be learned from an EQUIVALENCE of a function result -- there's no good reason to check such EQUIVALENCE statements for anything. I think that we should warn about their uselessness and otherwise ignore them.

[PeteSteinfeld]

In D92501#2429227, @klausler wrote:

I don't think that moving a temporary data structure from declaration processing into a more permanent object like Scope to accomplish scoping is the right way to fix this problem, unfortunately.

The specification-part of an interface block exists only to define the dummy arguments and (if there is one) the function result, so declarations of other local variables are moot in an interface outside of specification inquiries like SIZE(). Since local variables are the only objects that could meaningfully appear in an EQUIVALENCE statement in the specification part of an interface -- dummies can't be EQUIVALENCEd and there's nothing to be learned from an EQUIVALENCE of a function result -- there's no good reason to check such EQUIVALENCE statements for anything. I think that we should warn about their uselessness and otherwise ignore them.

Thanks, Peter. I thought about doing it that way, but I rejected it since that makes it impossible to detect errors in an EQUIVALENCE statement that appears in an interface block, moot or not. But I'm happy to follow your reasoning. My plan is now to avoid semantic checking on EQUIVALENCE statements when analyzing the specification part of a procedure in an interface block. This means that errors in such EQUIVALENCE statements will go undetected. But, as you say, such errors are irrelevant to the program.

Stop me if you think this is the wrong strategy for fixing this problem.

[PeteSteinfeld]

I changed my implementation to just not do semantic checking for EQUIVALENCE
statements when in an interface block.

[klausler]

A warning message should be considered, too.

flang/lib/Semantics/resolve-names.cpp

if (const auto *statement{std::get_if<
             parser::Statement<common::Indirection<parser::StmtFunctionStmt>>>(
             &decl.u)}) {
       AnalyzeStmtFunctionStmt(statement->statement.value());
     }
   }
   // TODO: what about instantiations in BLOCK?
   CheckSaveStmts();
   CheckCommonBlocks();
+  if (!inInterfaceBlock()) {
+    // TODO: warn for the case where the EQUIVALENCE statement is in a
+    // procedure declaration in an interface block
     CheckEquivalenceSets();
   }
+}
 
 // Analyze the bodies of statement functions now that the symbols in this
 // specification part have been fully declared and implicitly typed.
 void ResolveNamesVisitor::AnalyzeStmtFunctionStmt(
     const parser::StmtFunctionStmt &stmtFunc) {
   Symbol *symbol{std::get<parser::Name>(stmtFunc.t).symbol};
   if (!symbol || !symbol->has<SubprogramDetails>()) {
     return;

flang/test/Semantics/equivalence01.f90

-! RUN: %S/test_errors.sh %s %t %f18
+!RUN: %S/test_errors.sh %s %t %f18
 subroutine s1
   integer i, j
   real r(2)
   !ERROR: Equivalence set must have more than one object
   equivalence(i, j),(r(1))
 end
 
 subroutine s2

 end
 
 module s15
   real :: a(2), b(2)
   equivalence(a(2),b(1))
   !ERROR: 'a(3_8)' and 'a(1_8)' cannot have the same first storage unit
   equivalence(b(2),a(1))
 end module
+
+subroutine s16
+
+  integer var, dupName
+
+  ! There should be no error message for the following
+  equivalence (dupName, var)
+
+  interface
+    subroutine interfaceSub (dupName)
+      integer dupName
+    end subroutine interfaceSub
+  end interface
+
+end subroutine s16