[flang] More changes in response to review comments.

 - resolve-names.cc: I reworded the message when a name appears in a
   locality-spec when a name is used that cannot appear in a variable
   definition context.
 - tools.cc: I removed the unused functions ```IsValueDummy()``` and
   ```IsModifiable()```.  I made the function ```GetAssociatedVariable()```
   static.  I cleaned up the code in ```GetAssociationRoot()```.  I cleaned up
   the code in ```IsOrContainsEventOrLockComponent()```.  I added a TODO to
   ```WhyNotModifiable()``` and made some other improvements to it.
 - tools.h: Removed some deleted and unnecessary functions.
 - I fixed up a couple of tests related to the changes in error messages.

Original-commit: flang-compiler/f18@47da8ff9c8c131a6fc7a4a6f4a8fa89c8b582b71
Reviewed-on: https://github.com/flang-compiler/f18/pull/596

diff --git a/flang/lib/semantics/resolve-names.cc b/flang/lib/semantics/resolve-names.cc
index 567249a..4fbf954 100644 (file)
--- a/flang/lib/semantics/resolve-names.cc
+++ b/flang/lib/semantics/resolve-names.cc
@@ -3853,8 +3853,8 @@ bool DeclarationVisitor::PassesLocalityChecks(
   if (std::optional<MessageFixedText> msg{
           WhyNotModifiable(symbol, currScope())}) {
     SayWithReason(name, symbol,
-        "'%s' must be able to appear in a variable definition context to "
-        "appear in a locality-spec"_err_en_US,
+        "'%s' may not appear in a locality-spec because it is not "
+        "definable"_err_en_US,
         std::move(*msg));
     return false;
   }

diff --git a/flang/lib/semantics/tools.cc b/flang/lib/semantics/tools.cc
index ff26b1b..f3fabd3 100644 (file)
--- a/flang/lib/semantics/tools.cc
+++ b/flang/lib/semantics/tools.cc
@@ -124,10 +124,6 @@ bool IsDummy(const Symbol &symbol) {
   }
 }
 
-bool IsValueDummy(const Symbol &symbol) {
-  return IsDummy(symbol) && symbol.attrs().test(Attr::VALUE);
-}
-
 bool IsPointerDummy(const Symbol &symbol) {
   return IsPointer(symbol) && IsDummy(symbol);
 }
@@ -135,9 +131,7 @@ bool IsPointerDummy(const Symbol &symbol) {
 // variable-name
 bool IsVariableName(const Symbol &symbol) {
   if (const Symbol * root{GetAssociationRoot(symbol)}) {
-    return IsValueDummy(symbol) ||
-        (root->has<ObjectEntityDetails>() && !IsParameter(*root) &&
-            !IsIntentIn(*root));
+    return root->has<ObjectEntityDetails>() && !IsParameter(*root);
   } else {
     return false;
   }
@@ -337,7 +331,7 @@ const Symbol *FindFunctionResult(const Symbol &symbol) {
   return nullptr;
 }
 
-const Symbol *GetAssociatedVariable(const AssocEntityDetails &details) {
+static const Symbol *GetAssociatedVariable(const AssocEntityDetails &details) {
   if (const MaybeExpr & expr{details.expr()}) {
     if (evaluate::IsVariable(*expr)) {
       if (const Symbol * varSymbol{evaluate::GetLastSymbol(*expr)}) {
@@ -352,8 +346,9 @@ const Symbol *GetAssociatedVariable(const AssocEntityDetails &details) {
 // Return nullptr if the name is associated with an expression
 const Symbol *GetAssociationRoot(const Symbol &symbol) {
   const Symbol &ultimate{symbol.GetUltimate()};
-  if (ultimate.has<AssocEntityDetails>()) {  // construct association
-    return GetAssociatedVariable(ultimate.get<AssocEntityDetails>());
+  if (const auto *details{ultimate.detailsIf<AssocEntityDetails>()}) {
+    // We have a construct association
+    return GetAssociatedVariable(*details);
   } else {
     return &ultimate;
   }
@@ -435,10 +430,8 @@ bool IsOrContainsEventOrLockComponent(const Symbol &symbol) {
     if (const auto *details{root->detailsIf<ObjectEntityDetails>()}) {
       if (const DeclTypeSpec * type{details->type()}) {
         if (const DerivedTypeSpec * derived{type->AsDerived()}) {
-          if (IsEventTypeOrLockType(derived) ||
-              HasEventOrLockPotentialComponent(*derived)) {
-            return true;
-          }
+          return IsEventTypeOrLockType(derived) ||
+              HasEventOrLockPotentialComponent(*derived);
         }
       }
     }
@@ -493,7 +486,7 @@ bool IsAssumedSizeArray(const Symbol &symbol) {
 bool IsExternalInPureContext(const Symbol &symbol, const Scope &scope) {
   if (const auto *pureProc{semantics::FindPureProcedureContaining(&scope)}) {
     if (const Symbol * root{GetAssociationRoot(symbol)}) {
-      if (semantics::FindExternallyVisibleObject(*root, *pureProc)) {
+      if (FindExternallyVisibleObject(*root, *pureProc)) {
         return true;
       }
     }
@@ -505,6 +498,9 @@ bool InProtectedContext(const Symbol &symbol, const Scope &currentScope) {
   return IsProtected(symbol) && !IsHostAssociated(symbol, currentScope);
 }
 
+// C1101 and C1158
+// TODO Need to check for the case of a variable that has a vector subscript
+// that is construct associated, also need to check for a coindexed object
 std::optional<parser::MessageFixedText> WhyNotModifiable(
     const Symbol &symbol, const Scope &scope) {
   const Symbol *root{GetAssociationRoot(symbol)};
@@ -513,10 +509,11 @@ std::optional<parser::MessageFixedText> WhyNotModifiable(
   } else if (InProtectedContext(*root, scope)) {
     return "'%s' is protected in this scope"_en_US;
   } else if (IsExternalInPureContext(*root, scope)) {
-    return "'%s' is an external that is referenced in a PURE procedure"_en_US;
+    return "'%s' is externally visible and referenced in a PURE"
+           " procedure"_en_US;
   } else if (IsOrContainsEventOrLockComponent(*root)) {
     return "'%s' is an entity with either an EVENT_TYPE or LOCK_TYPE"_en_US;
-  } else if (IsIntentIn(*root) && !IsValueDummy(*root)) {
+  } else if (IsIntentIn(*root)) {
     return "'%s' is an INTENT(IN) dummy argument"_en_US;
   } else if (!IsVariableName(*root)) {
     return "'%s' is not a variable"_en_US;
@@ -525,10 +522,6 @@ std::optional<parser::MessageFixedText> WhyNotModifiable(
   }
 }
 
-bool IsModifiable(const Symbol &symbol, const Scope &scope) {
-  return !WhyNotModifiable(symbol, scope);
-}
-
 static const DeclTypeSpec &InstantiateIntrinsicType(Scope &scope,
     const DeclTypeSpec &spec, SemanticsContext &semanticsContext) {
   const IntrinsicTypeSpec *intrinsic{spec.AsIntrinsic()};

diff --git a/flang/lib/semantics/tools.h b/flang/lib/semantics/tools.h
index 7b6e355..6cb8f4a 100644 (file)
--- a/flang/lib/semantics/tools.h
+++ b/flang/lib/semantics/tools.h
@@ -46,7 +46,6 @@ const Symbol *FindPointerComponent(const Symbol &);
 const Symbol *FindInterface(const Symbol &);
 const Symbol *FindSubprogram(const Symbol &);
 const Symbol *FindFunctionResult(const Symbol &);
-const Symbol *GetAssociatedVariable(const AssocEntityDetails &);
 const Symbol *GetAssociationRoot(const Symbol &);
 
 bool IsCommonBlockContaining(const Symbol &block, const Symbol &object);
@@ -56,7 +55,6 @@ bool IsUseAssociated(const Symbol *, const Scope &);
 bool IsHostAssociated(const Symbol &, const Scope &);
 bool IsDummy(const Symbol &);
 bool IsPointerDummy(const Symbol &);
-bool IsValueDummy(const Symbol &);
 bool IsFunction(const Symbol &);
 bool IsPureProcedure(const Symbol &);
 bool IsPureProcedure(const Scope &);
@@ -116,7 +114,6 @@ bool IsAssumedSizeArray(const Symbol &symbol);
 std::optional<parser::MessageFixedText> WhyNotModifiable(
     const Symbol &symbol, const Scope &scope);
 // Is the symbol modifiable in this scope
-bool IsModifiable(const Symbol &symbol, const Scope &scope);
 bool IsExternalInPureContext(const Symbol &symbol, const Scope &scope);
 
 // Returns the complete list of derived type parameter symbols in

diff --git a/flang/test/semantics/resolve35.f90 b/flang/test/semantics/resolve35.f90
index 387787a..0d9f8a0 100644 (file)
--- a/flang/test/semantics/resolve35.f90
+++ b/flang/test/semantics/resolve35.f90
@@ -119,13 +119,13 @@ subroutine s10
   real, parameter :: bad2 = 1.0
   x = cos(0.)
   do concurrent(i=1:2) &
-    !ERROR: 'bad1' must be able to appear in a variable definition context to appear in a locality-spec
+    !ERROR: 'bad1' may not appear in a locality-spec because it is not definable
     local(bad1) &
-    !ERROR: 'bad2' must be able to appear in a variable definition context to appear in a locality-spec
+    !ERROR: 'bad2' may not appear in a locality-spec because it is not definable
     local(bad2) &
-    !ERROR: 'bad3' must be able to appear in a variable definition context to appear in a locality-spec
+    !ERROR: 'bad3' may not appear in a locality-spec because it is not definable
     local(bad3) &
-    !ERROR: 'cos' must be able to appear in a variable definition context to appear in a locality-spec
+    !ERROR: 'cos' may not appear in a locality-spec because it is not definable
     local(cos)
   end do
   do concurrent(i=1:2) &

diff --git a/flang/test/semantics/resolve57.f90 b/flang/test/semantics/resolve57.f90
index ae9a522..2892b24 100644 (file)
--- a/flang/test/semantics/resolve57.f90
+++ b/flang/test/semantics/resolve57.f90
@@ -52,7 +52,7 @@ subroutine s4()
   use m3
 
   ! C857 This is not OK because of the "protected" attribute
-!ERROR: 'prot' must be able to appear in a variable definition context to appear in a locality-spec
+!ERROR: 'prot' may not appear in a locality-spec because it is not definable
   do concurrent (i=1:5) local(prot)
   end do
 
@@ -71,7 +71,7 @@ subroutine s5()
     end do
 
     ! C1101 This is not OK because 'a' is not associated with a variable
-!ERROR: 'a' must be able to appear in a variable definition context to appear in a locality-spec
+!ERROR: 'a' may not appear in a locality-spec because it is not definable
     do concurrent (i=1:5) local(a)
     end do
   end associate
@@ -100,7 +100,7 @@ subroutine s6()
   select type ( a => func() )
   type is ( point )
     ! C1158 This is not OK because 'a' is not associated with a variable
-!ERROR: 'a' must be able to appear in a variable definition context to appear in a locality-spec
+!ERROR: 'a' may not appear in a locality-spec because it is not definable
     do concurrent (i=1:5) local(a)
     end do
   end select
@@ -121,7 +121,7 @@ pure subroutine s7()
   use m4
 
   ! C1594 This is not OK because we're in a PURE subroutine
-!ERROR: 'var' must be able to appear in a variable definition context to appear in a locality-spec
+!ERROR: 'var' may not appear in a locality-spec because it is not definable
   do concurrent (i=1:5) local(var)
   end do
 end subroutine s7
@@ -129,7 +129,7 @@ end subroutine s7
 subroutine s8()
   integer, parameter :: iconst = 343
 
-!ERROR: 'iconst' must be able to appear in a variable definition context to appear in a locality-spec
+!ERROR: 'iconst' may not appear in a locality-spec because it is not definable
   do concurrent (i=1:5) local(iconst)
   end do
 end subroutine s8