[flang] Fix erroneous application of SAVE statement

[tskeith]

Summary

A SAVE statement with no entity list applies the SAVE attribute only to
the entities that it is allowed on. We were applying it to automatic
data objects and reporting an error that they can't have SAVE.

The fix is to change DeclarationVisitor::CheckSaveAttr to check for
automatic objects. That controls both checking and setting the
attribute. This allows us to remove the check from CheckSpecExpr
(along with symbolBeingChecked_). Also, it was only called on constant
objects so the non-const overload can be eliminated.

The check in CheckSpecExpr is replaced by an explicit check for
automatic objects in modules. This caught an error in modfile03.f90 so
that part of the test was eliminated.

[klausler]

LGTM, thanks.

flang/lib/Semantics/check-declarations.cpp

public:
   }
   void Check(const ArraySpec &);
   void Check(const DeclTypeSpec &, bool canHaveAssumedTypeParameters);
   void Check(const Symbol &);
   void Check(const Scope &);
 
 private:
   template <typename A> void CheckSpecExpr(const A &x) {
-    if (symbolBeingChecked_ && IsSaved(*symbolBeingChecked_)) {
-      if (!evaluate::IsConstantExpr(x)) {
-        messages_.Say(
-            "Specification expression must be constant in declaration of '%s' with the SAVE attribute"_err_en_US,
-            symbolBeingChecked_->name());
-      }
-    } else {
-      evaluate::CheckSpecificationExpr(
-          x, messages_, DEREF(scope_), context_.intrinsics());
-    }
-  }
-  template <typename A> void CheckSpecExpr(const std::optional<A> &x) {
-    if (x) {
-      CheckSpecExpr(*x);
-    }
-  }
-  template <typename A> void CheckSpecExpr(A &x) {
-    x = Fold(foldingContext_, std::move(x));
-    const A &constx{x};
-    CheckSpecExpr(constx);
-  }
   void CheckValue(const Symbol &, const DerivedTypeSpec *);
   void CheckVolatile(
       const Symbol &, bool isAssociated, const DerivedTypeSpec *);
   void CheckPointer(const Symbol &);
   void CheckPassArg(
       const Symbol &proc, const Symbol *interface, const WithPassArg &);
   void CheckProcBinding(const Symbol &, const ProcBindingDetails &);
   void CheckObjectEntity(const Symbol &, const ObjectEntityDetails &);
private:
 
   SemanticsContext &context_;
   evaluate::FoldingContext &foldingContext_{context_.foldingContext()};
   parser::ContextualMessages &messages_{foldingContext_.messages()};
   const Scope *scope_{nullptr};
   // This symbol is the one attached to the innermost enclosing scope
   // that has a symbol.
   const Symbol *innermostSymbol_{nullptr};
-  const Symbol *symbolBeingChecked_{nullptr};
 };
 
 void CheckHelper::Check(const ParamValue &value, bool canBeAssumed) {
   if (value.isAssumed()) {
     if (!canBeAssumed) { // C795, C721, C726
       messages_.Say(
           "An assumed (*) type parameter may be used only for a (non-statement"
           " function) dummy argument, associate name, named constant, or"
if (IsDummy(symbol)) {
     }
   }
   if (symbol.owner().IsDerivedType() &&
       (symbol.attrs().test(Attr::CONTIGUOUS) &&
           !(IsPointer(symbol) && symbol.Rank() > 0))) { // C752
     messages_.Say(
         "A CONTIGUOUS component must be an array with the POINTER attribute"_err_en_US);
   }
+  if (symbol.owner().IsModule() && IsAutomatic(symbol)) {
+    messages_.Say(
+        "Automatic data object '%s' may not appear in the specification part"
+        " of a module"_err_en_US,
+        symbol.name());
+  }
 }
 
 void CheckHelper::CheckValue(
     const Symbol &symbol, const DerivedTypeSpec *derived) { // C863 - C865
   if (!IsDummy(symbol)) {
     messages_.Say(
         "VALUE attribute may apply only to a dummy argument"_err_en_US);
   }
if (!IsDummy(symbol)) {
             symbol.name());
       }
     }
   }
 }
 
 void CheckHelper::CheckObjectEntity(
     const Symbol &symbol, const ObjectEntityDetails &details) {
-  CHECK(!symbolBeingChecked_);
-  symbolBeingChecked_ = &symbol; // for specification expr checks
   CheckArraySpec(symbol, details.shape());
   Check(details.shape());
   Check(details.coshape());
   CheckAssumedTypeEntity(symbol, details);
-  symbolBeingChecked_ = nullptr;
   if (!details.coshape().empty()) {
     bool isDeferredShape{details.coshape().IsDeferredShape()};
     if (IsAllocatable(symbol)) {
       if (!isDeferredShape) { // C827
         messages_.Say("'%s' is an ALLOCATABLE coarray and must have a deferred"
                       " coshape"_err_en_US,
             symbol.name());
       }

flang/lib/Semantics/resolve-names.cpp

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.
 Attrs DeclarationVisitor::HandleSaveName(const SourceName &name, Attrs attrs) {
   if (attrs.test(Attr::SAVE)) {

flang/test/Semantics/modfile03.f90

 ! pure function f1(i)
 !  integer(4),value::i
 !  integer(4)::f1
 ! end
 !end
 
 module m5b
   use m5a, only: k2 => k1, l2 => l1, f2 => f1
-  character(l2, k2) :: x
   interface
     subroutine s(x, y)
       import f2, l2
       character(l2, k2) :: x
       character(f2(l2)) :: y
     end subroutine
   end interface
 end
 !Expect: m5b.mod
 !module m5b
 ! use m5a,only:k2=>k1
 ! use m5a,only:l2=>l1
 ! use m5a,only:f2=>f1
-! character(l2,4)::x
 ! interface
 !  subroutine s(x,y)
 !   import::f2
 !   import::l2
 !   character(l2,4)::x
 !   character(f2(l2),1)::y
 !  end
 ! end interface

flang/test/Semantics/resolve45.f90

subroutine s6
   !ERROR: SAVE attribute was already specified on 'z'
   save x,z
 end
 
 subroutine s7
   !ERROR: 'x' appears as a COMMON block in a SAVE statement but not in a COMMON statement
   save /x/
 end
+
+subroutine s8a(n)
+  integer :: n
+  real :: x(n)  ! OK: save statement doesn't affect x
+  save
+end
+subroutine s8b(n)
+  integer :: n
+  !ERROR: SAVE attribute may not be applied to automatic data object 'x'
+  real, save :: x(n)
+end

flang/test/Semantics/resolve77.f90

 ! RUN: %S/test_errors.sh %s %t %f18
 ! Tests valid and invalid usage of forward references to procedures
 ! in specification expressions.
 module m
   interface ifn2
     module procedure if2
   end interface
   interface ifn3
     module procedure if3
   end interface
-  !ERROR: Specification expression must be constant in declaration of 'a' with the SAVE attribute
+  !ERROR: Automatic data object 'a' may not appear in the specification part of a module
   real :: a(if1(1))
   !ERROR: No specific procedure of generic 'ifn2' matches the actual arguments
   real :: b(ifn2(1))
  contains
   subroutine t1(n)
     integer :: iarr(if1(n))
   end subroutine
   pure integer function if1(n)