[flang-commits] [flang] 3f6e0c2 - [flang] Move SAVE attribute checks to declaration checking

Peter Klausler via flang-commits flang-commits at lists.llvm.org
Mon Mar 27 16:01:44 PDT 2023


Author: Peter Klausler
Date: 2023-03-27T15:53:44-07:00
New Revision: 3f6e0c24e6a7190f309bb44a9e61f8d8fd559d11

URL: https://github.com/llvm/llvm-project/commit/3f6e0c24e6a7190f309bb44a9e61f8d8fd559d11
DIFF: https://github.com/llvm/llvm-project/commit/3f6e0c24e6a7190f309bb44a9e61f8d8fd559d11.diff

LOG: [flang] Move SAVE attribute checks to declaration checking

Constraint checking for explicit SAVE attributes is more
accurate when done along with other declaration checking, rather
than on the fly during name resolution.  This allows us to
catch attempts to attach explicit SAVE attributes to anything
that can't have one (constraints C859, C860).

Also delete `IsSave()`, whose few remaining uses were changed to the
more general `IsSaved()` predicate that seems more correct for
those uses, returning true for both explicit and implied SAVE
attributes.

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

Added: 
    

Modified: 
    flang/include/flang/Semantics/tools.h
    flang/lib/Semantics/check-declarations.cpp
    flang/lib/Semantics/check-omp-structure.cpp
    flang/lib/Semantics/resolve-names-utils.cpp
    flang/lib/Semantics/resolve-names.cpp
    flang/test/Lower/host-associated-globals.f90
    flang/test/Semantics/resolve45.f90

Removed: 
    


################################################################################
diff  --git a/flang/include/flang/Semantics/tools.h b/flang/include/flang/Semantics/tools.h
index 443c9455ee4a4..ce78282940629 100644
--- a/flang/include/flang/Semantics/tools.h
+++ b/flang/include/flang/Semantics/tools.h
@@ -145,9 +145,6 @@ inline bool IsAllocatable(const Symbol &symbol) {
 inline bool IsAllocatableOrPointer(const Symbol &symbol) {
   return IsPointer(symbol) || IsAllocatable(symbol);
 }
-inline bool IsSave(const Symbol &symbol) {
-  return symbol.attrs().test(Attr::SAVE);
-}
 inline bool IsNamedConstant(const Symbol &symbol) {
   return symbol.attrs().test(Attr::PARAMETER);
 }

diff  --git a/flang/lib/Semantics/check-declarations.cpp b/flang/lib/Semantics/check-declarations.cpp
index ee6853fdeb432..9b5d28cf574bd 100644
--- a/flang/lib/Semantics/check-declarations.cpp
+++ b/flang/lib/Semantics/check-declarations.cpp
@@ -115,6 +115,7 @@ class CheckHelper {
   }
   bool IsResultOkToDiffer(const FunctionResult &);
   void CheckGlobalName(const Symbol &);
+  void CheckExplicitSave(const Symbol &);
   void CheckBindC(const Symbol &);
   void CheckBindCFunctionResult(const Symbol &);
   // Check functions for defined I/O procedures
@@ -257,6 +258,10 @@ void CheckHelper::Check(const Symbol &symbol) {
   if (symbol.attrs().test(Attr::BIND_C)) {
     CheckBindC(symbol);
   }
+  if (symbol.attrs().test(Attr::SAVE) &&
+      !symbol.implicitAttrs().test(Attr::SAVE)) {
+    CheckExplicitSave(symbol);
+  }
   CheckGlobalName(symbol);
   if (isDone) {
     return; // following checks do not apply
@@ -399,21 +404,11 @@ void CheckHelper::Check(const Symbol &symbol) {
       messages_.Say(
           "A dummy argument may not also be a named constant"_err_en_US);
     }
-    if (!symbol.test(Symbol::Flag::InDataStmt) /*caught elsewhere*/ &&
-        IsSaved(symbol)) {
-      messages_.Say(
-          "A dummy argument may not have the SAVE attribute"_err_en_US);
-    }
   } else if (IsFunctionResult(symbol)) {
     if (IsNamedConstant(symbol)) {
       messages_.Say(
           "A function result may not also be a named constant"_err_en_US);
     }
-    if (!symbol.test(Symbol::Flag::InDataStmt) /*caught elsewhere*/ &&
-        IsSaved(symbol)) {
-      messages_.Say(
-          "A function result may not have the SAVE attribute"_err_en_US);
-    }
     CheckBindCFunctionResult(symbol);
   }
   if (symbol.owner().IsDerivedType() &&
@@ -453,6 +448,38 @@ void CheckHelper::CheckCommonBlock(const Symbol &symbol) {
   }
 }
 
+// C859, C860
+void CheckHelper::CheckExplicitSave(const Symbol &symbol) {
+  const Symbol &ultimate{symbol.GetUltimate()};
+  if (ultimate.test(Symbol::Flag::InDataStmt)) {
+    // checked elsewhere
+  } else if (symbol.has<UseDetails>()) {
+    messages_.Say(
+        "The USE-associated name '%s' may not have an explicit SAVE attribute"_err_en_US,
+        symbol.name());
+  } else if (IsDummy(ultimate)) {
+    messages_.Say(
+        "The dummy argument '%s' may not have an explicit SAVE attribute"_err_en_US,
+        symbol.name());
+  } else if (IsFunctionResult(ultimate)) {
+    messages_.Say(
+        "The function result variable '%s' may not have an explicit SAVE attribute"_err_en_US,
+        symbol.name());
+  } else if (const Symbol * common{FindCommonBlockContaining(ultimate)}) {
+    messages_.Say(
+        "The entity '%s' in COMMON block /%s/ may not have an explicit SAVE attribute"_err_en_US,
+        symbol.name(), common->name());
+  } else if (IsAutomatic(ultimate)) {
+    messages_.Say(
+        "The automatic object '%s' may not have an explicit SAVE attribute"_err_en_US,
+        symbol.name());
+  } else if (!evaluate::IsVariable(ultimate) && !IsProcedurePointer(ultimate)) {
+    messages_.Say(
+        "The entity '%s' with an explicit SAVE attribute must be a variable, procedure pointer, or COMMON block"_err_en_US,
+        symbol.name());
+  }
+}
+
 void CheckHelper::CheckBindCFunctionResult(const Symbol &symbol) { // C1553
   if (!innermostSymbol_ || !IsBindCProcedure(*innermostSymbol_)) {
     return;
@@ -954,10 +981,6 @@ void CheckHelper::CheckProcEntity(
             symbol.name()); // C1517
       }
     }
-  } else if (IsSave(symbol)) {
-    messages_.Say(
-        "Procedure '%s' with SAVE attribute must also have POINTER attribute"_err_en_US,
-        symbol.name());
   }
   CheckExternal(symbol);
 }

diff  --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index cffdabf8441d0..9794424e266d6 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -284,7 +284,7 @@ void OmpStructureChecker::CheckPredefinedAllocatorRestriction(
     const auto &scope{context_.FindScope(symbol->name())};
     const Scope &containingScope{GetProgramUnitContaining(scope)};
     if (!isPredefinedAllocator &&
-        (IsSave(*symbol) || commonBlock ||
+        (IsSaved(*symbol) || commonBlock ||
             containingScope.kind() == Scope::Kind::Module)) {
       context_.Say(source,
           "If list items within the ALLOCATE directive have the "
@@ -1026,7 +1026,7 @@ void OmpStructureChecker::CheckThreadprivateOrDeclareTargetVar(
                           "%s "
                           "directive"_err_en_US,
                           ContextDirectiveAsFortran());
-                    } else if (!IsSave(*name->symbol) &&
+                    } else if (!IsSaved(*name->symbol) &&
                         declScope.kind() != Scope::Kind::MainProgram &&
                         declScope.kind() != Scope::Kind::Module) {
                       context_.Say(name->source,

diff  --git a/flang/lib/Semantics/resolve-names-utils.cpp b/flang/lib/Semantics/resolve-names-utils.cpp
index bd628f47e26c2..a5b50a81f08f9 100644
--- a/flang/lib/Semantics/resolve-names-utils.cpp
+++ b/flang/lib/Semantics/resolve-names-utils.cpp
@@ -357,8 +357,14 @@ Bound ArraySpecAnalyzer::GetBound(const parser::SpecificationExpr &x) {
 static void PropagateSaveAttr(
     const EquivalenceObject &src, EquivalenceSet &dst) {
   if (src.symbol.attrs().test(Attr::SAVE)) {
+    bool isImplicit{src.symbol.implicitAttrs().test(Attr::SAVE)};
     for (auto &obj : dst) {
-      obj.symbol.attrs().set(Attr::SAVE);
+      if (!obj.symbol.attrs().test(Attr::SAVE)) {
+        obj.symbol.attrs().set(Attr::SAVE);
+        if (isImplicit) {
+          obj.symbol.implicitAttrs().set(Attr::SAVE);
+        }
+      }
     }
   }
 }

diff  --git a/flang/lib/Semantics/resolve-names.cpp b/flang/lib/Semantics/resolve-names.cpp
index 12370b08a3bfd..341bde8baab4f 100644
--- a/flang/lib/Semantics/resolve-names.cpp
+++ b/flang/lib/Semantics/resolve-names.cpp
@@ -1097,10 +1097,8 @@ class DeclarationVisitor : public ArraySpecVisitor,
   ParamValue GetParamValue(
       const parser::TypeParamValue &, common::TypeParamAttr attr);
   void CheckCommonBlockDerivedType(const SourceName &, const Symbol &);
-  std::optional<MessageFixedText> CheckSaveAttr(const Symbol &);
   Attrs HandleSaveName(const SourceName &, Attrs);
   void AddSaveName(std::set<SourceName> &, const SourceName &);
-  void SetSaveAttr(Symbol &);
   bool HandleUnrestrictedSpecificIntrinsicFunction(const parser::Name &);
   const parser::Name *FindComponent(const parser::Name *, const parser::Name &);
   void Initialization(const parser::Name &, const parser::Initialization &,
@@ -2877,7 +2875,7 @@ void ModuleVisitor::DoAddUse(SourceName location, SourceName localName,
   if (localSymbol.has<UnknownDetails>()) {
     localSymbol.set_details(UseDetails{localName, useSymbol});
     localSymbol.attrs() =
-        useSymbol.attrs() & ~Attrs{Attr::PUBLIC, Attr::PRIVATE};
+        useSymbol.attrs() & ~Attrs{Attr::PUBLIC, Attr::PRIVATE, Attr::SAVE};
     localSymbol.implicitAttrs() =
         localSymbol.attrs() & Attrs{Attr::ASYNCHRONOUS, Attr::VOLATILE};
     localSymbol.flags() = useSymbol.flags();
@@ -5595,11 +5593,8 @@ void DeclarationVisitor::CheckSaveStmts() {
       Say2(name,
           "Explicit SAVE of '%s' is redundant due to global SAVE statement"_warn_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);
+    } else if (!IsSaved(*symbol)) {
+      SetExplicitAttr(*symbol, Attr::SAVE);
     }
   }
   for (const SourceName &name : specPartState_.saveInfo.commons) {
@@ -5617,40 +5612,16 @@ void DeclarationVisitor::CheckSaveStmts() {
         }
       } else {
         for (auto &object : symbol->get<CommonBlockDetails>().objects()) {
-          SetSaveAttr(*object);
+          if (!IsSaved(*object)) {
+            SetImplicitAttr(*object, Attr::SAVE);
+          }
         }
       }
     }
   }
-  if (specPartState_.saveInfo.saveAll) {
-    // Apply SAVE attribute to applicable symbols
-    for (auto pair : currScope()) {
-      auto &symbol{*pair.second};
-      if (!CheckSaveAttr(symbol)) {
-        SetSaveAttr(symbol);
-      }
-    }
-  }
   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 specPartState_.saveInfo.entities.
 Attrs DeclarationVisitor::HandleSaveName(const SourceName &name, Attrs attrs) {
   if (attrs.test(Attr::SAVE)) {
@@ -5669,13 +5640,6 @@ void DeclarationVisitor::AddSaveName(
   }
 }
 
-// Set the SAVE attribute on symbol unless it is implicitly saved anyway.
-void DeclarationVisitor::SetSaveAttr(Symbol &symbol) {
-  if (!IsSaved(symbol)) {
-    SetImplicitAttr(symbol, Attr::SAVE);
-  }
-}
-
 // Check types of common block objects, now that they are known.
 void DeclarationVisitor::CheckCommonBlocks() {
   // check for empty common blocks

diff  --git a/flang/test/Lower/host-associated-globals.f90 b/flang/test/Lower/host-associated-globals.f90
index 2899f82ad6289..1f23b15fe3af4 100644
--- a/flang/test/Lower/host-associated-globals.f90
+++ b/flang/test/Lower/host-associated-globals.f90
@@ -26,9 +26,9 @@ subroutine bar()
 ! CHECK:  %[[VAL_4:.*]] = fir.address_of(@_QMtest_mod_used_in_hostEnot_in_equiv) : !fir.ref<i32>
 
 subroutine test_common()
-  integer, save :: i(2)
-  integer, save :: j_in_equiv
-  integer, save :: not_in_equiv
+  integer :: i(2)
+  integer :: j_in_equiv
+  integer :: not_in_equiv
   equivalence (i(2),j_in_equiv)
   common /x/ i, not_in_equiv
   call bar()

diff  --git a/flang/test/Semantics/resolve45.f90 b/flang/test/Semantics/resolve45.f90
index 61e766356541f..7f6d8d6ae50b2 100644
--- a/flang/test/Semantics/resolve45.f90
+++ b/flang/test/Semantics/resolve45.f90
@@ -1,22 +1,24 @@
 ! RUN: %python %S/test_errors.py %s %flang_fc1
+!ERROR: The function result variable 'f1' may not have an explicit SAVE attribute
 function f1(x, y)
+  !ERROR: The dummy argument 'x' may not have an explicit SAVE attribute
   integer x
-  !ERROR: SAVE attribute may not be applied to dummy argument 'x'
-  !ERROR: SAVE attribute may not be applied to dummy argument 'y'
   save x,y
+  !ERROR: The dummy argument 'y' may not have an explicit SAVE attribute
   integer y
-  !ERROR: SAVE attribute may not be applied to function result 'f1'
   save f1
 end
 
-function f2(x, y)
-  !ERROR: SAVE attribute may not be applied to function result 'f2'
-  real, save :: f2
-  !ERROR: SAVE attribute may not be applied to dummy argument 'x'
+!ERROR: The entity 'f2' with an explicit SAVE attribute must be a variable, procedure pointer, or COMMON block
+function f2(x, y) result(r)
+  save f2
+  !ERROR: The function result variable 'r' may not have an explicit SAVE attribute
+  real, save :: r
+  !ERROR: The dummy argument 'x' may not have an explicit SAVE attribute
   complex, save :: x
   allocatable :: y
+  !ERROR: The dummy argument 'y' may not have an explicit SAVE attribute
   integer :: y
-  !ERROR: SAVE attribute may not be applied to dummy argument 'y'
   save :: y
 end
 
@@ -27,9 +29,9 @@ function f2b(x, y)
 end
 
 subroutine s3(x)
-  !ERROR: SAVE attribute may not be applied to dummy argument 'x'
+  !ERROR: The dummy argument 'x' may not have an explicit SAVE attribute
   procedure(integer), pointer, save :: x
-  !ERROR: Procedure 'y' with SAVE attribute must also have POINTER attribute
+  !ERROR: The entity 'y' with an explicit SAVE attribute must be a variable, procedure pointer, or COMMON block
   procedure(integer), save :: y
 end
 
@@ -65,6 +67,6 @@ subroutine s8a(n)
 end
 subroutine s8b(n)
   integer :: n
-  !ERROR: SAVE attribute may not be applied to automatic data object 'x'
+  !ERROR: The automatic object 'x' may not have an explicit SAVE attribute
   real, save :: x(n)
 end


        


More information about the flang-commits mailing list