[flang] Manage per-specification-part state better

[klausler]

Summary

Some state in name resolution is stored in the DeclarationVisitor
instance and processed at the end of the specification part.
This state needs to accommodate nested specification parts, namely
the ones that can be nested in a subroutine or function interface
body.

flang/lib/Semantics/resolve-names.cpp

const DeclTypeSpec &MakeLogicalType(
       const std::optional<parser::KindSelector> &);
   void NotePossibleBadForwardRef(const parser::Name &);
   std::optional<SourceName> HadForwardRef(const Symbol &) const;
   bool CheckPossibleBadForwardRef(const Symbol &);
 
   bool inExecutionPart_{false};
   bool inSpecificationPart_{false};
   bool inEquivalenceStmt_{false};
-  std::set<SourceName> specPartForwardRefs_;
+
+  // Some information is collected from a specification part for deferred
+  // processing in DeclarationPartVisitor functions (e.g., CheckSaveStmts())
+  // that are called by ResolveNamesVisitor::FinishSpecificationPart().  Since
+  // specification parts can nest (e.g., INTERFACE bodies), the collected
+  // information that is not contained in the scope needs to be packaged
+  // and restorable.
+  struct SpecificationPartState {
+    std::set<SourceName> forwardRefs;
+    // Collect equivalence sets and process at end of specification part
+    std::vector<const std::list<parser::EquivalenceObject> *> equivalenceSets;
+    // Names of all common block objects in the scope
+    std::set<SourceName> commonBlockObjects;
+    // Info about about SAVE statements and attributes in current scope
+    struct {
+      std::optional<SourceName> saveAll; // "SAVE" without entity list
+      std::set<SourceName> entities; // names of entities with save attr
+      std::set<SourceName> commons; // names of common blocks with save attr
+    } saveInfo;
+  } specPartState_;
 
 private:
   Scope *currScope_{nullptr};
 };
 
 class ModuleVisitor : public virtual ScopeHandler {
 public:
   bool Pre(const parser::AccessStmt &);
private:
   struct {
     const parser::Name *extends{nullptr}; // EXTENDS(name)
     bool privateComps{false}; // components are private by default
     bool privateBindings{false}; // bindings are private by default
     bool sawContains{false}; // currently processing bindings
     bool sequence{false}; // is a sequence type
     const Symbol *type{nullptr}; // derived type being defined
   } derivedTypeInfo_;
-  // Collect equivalence sets and process at end of specification part
-  std::vector<const std::list<parser::EquivalenceObject> *> equivalenceSets_;
-  // Names of all common block objects in the scope
-  std::set<SourceName> commonBlockObjects_;
-  // Info about about SAVE statements and attributes in current scope
-  struct {
-    std::optional<SourceName> saveAll; // "SAVE" without entity list
-    std::set<SourceName> entities; // names of entities with save attr
-    std::set<SourceName> commons; // names of common blocks with save attr
-  } saveInfo_;
   // In a ProcedureDeclarationStmt or ProcComponentDefStmt, this is
   // the interface name, if any.
   const parser::Name *interfaceName_{nullptr};
   // Map type-bound generic to binding names of its specific bindings
   std::multimap<Symbol *, const parser::Name *> genericBindings_;
   // Info about current ENUM
   struct EnumeratorState {
     // Enum value must hold inside a C_INT (7.6.2).
void ScopeHandler::NotePossibleBadForwardRef(const parser::Name &name) {
   if (inSpecificationPart_ && name.symbol) {
     auto kind{currScope().kind()};
     if ((kind == Scope::Kind::Subprogram && !currScope().IsStmtFunction()) ||
         kind == Scope::Kind::Block) {
       bool isHostAssociated{&name.symbol->owner() == &currScope()
               ? name.symbol->has<HostAssocDetails>()
               : name.symbol->owner().Contains(currScope())};
       if (isHostAssociated) {
-        specPartForwardRefs_.insert(name.source);
+        specPartState_.forwardRefs.insert(name.source);
       }
     }
   }
 }
 
 std::optional<SourceName> ScopeHandler::HadForwardRef(
     const Symbol &symbol) const {
-  auto iter{specPartForwardRefs_.find(symbol.name())};
-  if (iter != specPartForwardRefs_.end()) {
+  auto iter{specPartState_.forwardRefs.find(symbol.name())};
+  if (iter != specPartState_.forwardRefs.end()) {
     return *iter;
   }
   return std::nullopt;
 }
 
 bool ScopeHandler::CheckPossibleBadForwardRef(const Symbol &symbol) {
   if (!context().HasError(symbol)) {
     if (auto fwdRef{HadForwardRef(symbol)}) {

 bool DeclarationVisitor::Pre(const parser::CommonBlockObject &) {
   BeginArraySpec();
   return true;
 }
 
 void DeclarationVisitor::Post(const parser::CommonBlockObject &x) {
   const auto &name{std::get<parser::Name>(x.t)};
   DeclareObjectEntity(name);
-  auto pair{commonBlockObjects_.insert(name.source)};
+  auto pair{specPartState_.commonBlockObjects.insert(name.source)};
   if (!pair.second) {
     const SourceName &prev{*pair.first};
     Say2(name.source, "'%s' is already in a COMMON block"_err_en_US, prev,
         "Previous occurrence of '%s' in a COMMON block"_en_US);
   }
 }
 
 bool DeclarationVisitor::Pre(const parser::EquivalenceStmt &x) {
   // save equivalence sets to be processed after specification part
   if (CheckNotInBlock("EQUIVALENCE")) { // C1107
     for (const std::list<parser::EquivalenceObject> &set : x.v) {
-      equivalenceSets_.push_back(&set);
+      specPartState_.equivalenceSets.push_back(&set);
     }
   }
   return false; // don't implicitly declare names yet
 }
 
 void DeclarationVisitor::CheckEquivalenceSets() {
   EquivalenceSets equivSets{context()};
   inEquivalenceStmt_ = true;
-  for (const auto *set : equivalenceSets_) {
+  for (const auto *set : specPartState_.equivalenceSets) {
     const auto &source{set->front().v.value().source};
     if (set->size() <= 1) { // R871
       Say(source, "Equivalence set must have more than one object"_err_en_US);
     }
     for (const parser::EquivalenceObject &object : *set) {
       const auto &designator{object.v.value()};
       // The designator was not resolved when it was encountered so do it now.
       // AnalyzeExpr causes array sections to be changed to substrings as needed
       Walk(designator);
       if (AnalyzeExpr(context(), designator)) {
         equivSets.AddToSet(designator);
       }
     }
     equivSets.FinishSet(source);
   }
   inEquivalenceStmt_ = false;
   for (auto &set : equivSets.sets()) {
     if (!set.empty()) {
       currScope().add_equivalenceSet(std::move(set));
     }
   }
-  equivalenceSets_.clear();
+  specPartState_.equivalenceSets.clear();
 }
 
 bool DeclarationVisitor::Pre(const parser::SaveStmt &x) {
   if (x.v.empty()) {
-    saveInfo_.saveAll = currStmtSource();
+    specPartState_.saveInfo.saveAll = currStmtSource();
     currScope().set_hasSAVE();
   } else {
     for (const parser::SavedEntity &y : x.v) {
       auto kind{std::get<parser::SavedEntity::Kind>(y.t)};
       const auto &name{std::get<parser::Name>(y.t)};
       if (kind == parser::SavedEntity::Kind::Common) {
         MakeCommonBlockSymbol(name);
-        AddSaveName(saveInfo_.commons, name.source);
+        AddSaveName(specPartState_.saveInfo.commons, name.source);
       } else {
         HandleAttributeStmt(Attr::SAVE, name);
       }
     }
   }
   return false;
 }
 
 void DeclarationVisitor::CheckSaveStmts() {
-  for (const SourceName &name : saveInfo_.entities) {
+  for (const SourceName &name : specPartState_.saveInfo.entities) {
     auto *symbol{FindInScope(name)};
     if (!symbol) {
       // error was reported
-    } else if (saveInfo_.saveAll) {
+    } else if (specPartState_.saveInfo.saveAll) {
       // C889 - note that pgi, ifort, xlf do not enforce this constraint
       Say2(name,
           "Explicit SAVE of '%s' is redundant due to global SAVE statement"_err_en_US,
-          *saveInfo_.saveAll, "Global SAVE statement"_en_US);
+          *specPartState_.saveInfo.saveAll, "Global SAVE statement"_en_US);
     } else if (auto msg{CheckSaveAttr(*symbol)}) {
       Say(name, std::move(*msg));
       context().SetError(*symbol);
     } else {
       SetSaveAttr(*symbol);
     }
   }
-  for (const SourceName &name : saveInfo_.commons) {
+  for (const SourceName &name : specPartState_.saveInfo.commons) {
     if (auto *symbol{currScope().FindCommonBlock(name)}) {
       auto &objects{symbol->get<CommonBlockDetails>().objects()};
       if (objects.empty()) {
         if (currScope().kind() != Scope::Kind::Block) {
           Say(name,
               "'%s' appears as a COMMON block in a SAVE statement but not in"
               " a COMMON statement"_err_en_US);
         } else { // C1108
           Say(name,
               "SAVE statement in BLOCK construct may not contain a"
               " common block name '%s'"_err_en_US);
         }
       } else {
         for (auto &object : symbol->get<CommonBlockDetails>().objects()) {
           SetSaveAttr(*object);
         }
       }
     }
   }
-  if (saveInfo_.saveAll) {
+  if (specPartState_.saveInfo.saveAll) {
     // Apply SAVE attribute to applicable symbols
     for (auto pair : currScope()) {
       auto &symbol{*pair.second};
       if (!CheckSaveAttr(symbol)) {
         SetSaveAttr(symbol);
       }
     }
   }
-  saveInfo_ = {};
+  specPartState_.saveInfo = {};
 }
 
 // If SAVE attribute can't be set on symbol, return error message.
 std::optional<MessageFixedText> DeclarationVisitor::CheckSaveAttr(
     const Symbol &symbol) {
   if (IsDummy(symbol)) {
     return "SAVE attribute may not be applied to dummy argument '%s'"_err_en_US;
   } else if (symbol.IsFuncResult()) {
     return "SAVE attribute may not be applied to function result '%s'"_err_en_US;
   } else if (symbol.has<ProcEntityDetails>() &&
       !symbol.attrs().test(Attr::POINTER)) {
     return "Procedure '%s' with SAVE attribute must also have POINTER attribute"_err_en_US;
   } else if (IsAutomatic(symbol)) {
     return "SAVE attribute may not be applied to automatic data object '%s'"_err_en_US;
   } else {
     return std::nullopt;
   }
 }
 
-// Record SAVEd names in saveInfo_.entities.
+// Record SAVEd names in specPartState_.saveInfo.entities.
 Attrs DeclarationVisitor::HandleSaveName(const SourceName &name, Attrs attrs) {
   if (attrs.test(Attr::SAVE)) {
-    AddSaveName(saveInfo_.entities, name);
+    AddSaveName(specPartState_.saveInfo.entities, name);
   }
   return attrs;
 }
 
 // Record a name in a set of those to be saved.
 void DeclarationVisitor::AddSaveName(
     std::set<SourceName> &set, const SourceName &name) {
   auto pair{set.insert(name)};
for (const auto &pair : currScope().commonBlocks()) {
     if (symbol.get<CommonBlockDetails>().objects().empty() &&
         symbol.attrs().test(Attr::BIND_C)) {
       Say(symbol.name(),
           "'%s' appears as a COMMON block in a BIND statement but not in"
           " a COMMON statement"_err_en_US);
     }
   }
   // check objects in common blocks
-  for (const auto &name : commonBlockObjects_) {
+  for (const auto &name : specPartState_.commonBlockObjects) {
     const auto *symbol{currScope().FindSymbol(name)};
     if (!symbol) {
       continue;
     }
     const auto &attrs{symbol->attrs()};
     if (attrs.test(Attr::ALLOCATABLE)) {
       Say(name,
           "ALLOCATABLE object '%s' may not appear in a COMMON block"_err_en_US);
if (attrs.test(Attr::ALLOCATABLE)) {
           Say(name,
               "Derived type '%s' in COMMON block must have the BIND or"
               " SEQUENCE attribute"_err_en_US);
         }
         CheckCommonBlockDerivedType(name, typeSymbol);
       }
     }
   }
-  commonBlockObjects_ = {};
+  specPartState_.commonBlockObjects = {};
 }
 
 Symbol &DeclarationVisitor::MakeCommonBlockSymbol(const parser::Name &name) {
   return Resolve(name, currScope().MakeCommonBlock(name.source));
 }
 Symbol &DeclarationVisitor::MakeCommonBlockSymbol(
     const std::optional<parser::Name> &name) {
   if (name) {
if (symbol.has<UnknownDetails>()) {
     return false;
   }
 }
 
 bool ResolveNamesVisitor::Pre(const parser::SpecificationPart &x) {
   const auto &[accDecls, ompDecls, compilerDirectives, useStmts, importStmts,
       implicitPart, decls] = x.t;
   auto flagRestorer{common::ScopedSet(inSpecificationPart_, true)};
+  auto stateRestorer{
+      common::ScopedSet(specPartState_, SpecificationPartState{})};
   Walk(accDecls);
   Walk(ompDecls);
   Walk(compilerDirectives);
   Walk(useStmts);
   Walk(importStmts);
   Walk(implicitPart);
-  auto setRestorer{
-      common::ScopedSet(specPartForwardRefs_, std::set<SourceName>{})};
   for (const auto &decl : decls) {
     if (const auto *spec{
             std::get_if<parser::SpecificationConstruct>(&decl.u)}) {
       PreSpecificationConstruct(*spec);
     }
   }
   Walk(decls);
   FinishSpecificationPart(decls);

flang/test/Semantics/resolve101.f90

+! RUN: %S/test_errors.sh %s %t %f18
+! Ensure that spurious errors do not arise from FinishSpecificationPart
+! checking on a nested specification part.
+real, save :: x
+interface
+  subroutine subr(x)
+    real, intent(in) :: x
+    ! SAVE attribute checking should not complain at the
+    ! end of this specification part about a dummy argument
+    ! having the SAVE attribute.
+  end subroutine
+end interface
+end