[flang-commits] [flang] 9b20007 - [flang] Fix combining cases of USE association & generic interfaces

Peter Klausler via flang-commits flang-commits at lists.llvm.org
Thu Apr 14 09:00:01 PDT 2022


Author: Peter Klausler
Date: 2022-04-14T08:59:52-07:00
New Revision: 9b200074b17af425bc4366b7b0c3ba74c1f5b89a

URL: https://github.com/llvm/llvm-project/commit/9b200074b17af425bc4366b7b0c3ba74c1f5b89a
DIFF: https://github.com/llvm/llvm-project/commit/9b200074b17af425bc4366b7b0c3ba74c1f5b89a.diff

LOG: [flang] Fix combining cases of USE association & generic interfaces

Fortran admits a few ways to have multiple symbols with the
same name in the same scope.  Two of them involve generic
interfaces (from INTERFACE or GENERIC, the syntax doesn't matter);
these are allowed to inhabit a scope with either a derived type or
a subprogram that is also a specific procedure of the generic.
(But not both a derived type and a subprogram; they could not
cohabit a scope anyway, generic or not.)

In cases of USE association, f18 needs to be capable of combining
use-associated generic interfaces with other use-associated entities.
Two generics get merged (this case was nearly correct); a generic
and a derived type can merge into a GenericDetails with a shadowed
derivedType(); and a generic can replace or ignore a use-associated
procedure of the same name so long as that procedure is already
one of its specifics.

Further, these modifications to the use-associated generic
interface must be made to a local copy of the symbol.  The previous
code was messing directly with the symbol in the module's scope.

The fix is basically a reimplementation of the member function
DoAddUse() in name resolution.

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

Added: 
    

Modified: 
    flang/lib/Semantics/resolve-names.cpp
    flang/lib/Semantics/symbol.cpp
    flang/test/Semantics/modfile07.f90
    flang/test/Semantics/resolve17.f90

Removed: 
    


################################################################################
diff  --git a/flang/lib/Semantics/resolve-names.cpp b/flang/lib/Semantics/resolve-names.cpp
index 8eff236a4a0d1..5b6628fbe7efc 100644
--- a/flang/lib/Semantics/resolve-names.cpp
+++ b/flang/lib/Semantics/resolve-names.cpp
@@ -644,7 +644,7 @@ class ModuleVisitor : public virtual ScopeHandler {
   void BeginModule(const parser::Name &, bool isSubmodule);
   bool BeginSubmodule(const parser::Name &, const parser::ParentIdentifier &);
   void ApplyDefaultAccess();
-  void AddGenericUse(GenericDetails &, const SourceName &, const Symbol &);
+  Symbol &AddGenericUse(GenericDetails &, const SourceName &, const Symbol &);
   void AddAndCheckExplicitIntrinsicUse(SourceName, bool isIntrinsic);
   void ClearUseRenames() { useRenames_.clear(); }
   void ClearUseOnly() { useOnly_.clear(); }
@@ -678,8 +678,8 @@ class ModuleVisitor : public virtual ScopeHandler {
   // Record a use from useModuleScope_ of use Name/Symbol as local Name/Symbol
   SymbolRename AddUse(const SourceName &localName, const SourceName &useName);
   SymbolRename AddUse(const SourceName &, const SourceName &, Symbol *);
-  void DoAddUse(const SourceName &, const SourceName &, Symbol &localSymbol,
-      const Symbol &useSymbol);
+  void DoAddUse(
+      SourceName, SourceName, Symbol &localSymbol, const Symbol &useSymbol);
   void AddUse(const GenericSpecInfo &);
   // If appropriate, erase a previously USE-associated symbol
   void EraseRenamedSymbol(const Symbol &);
@@ -2608,70 +2608,147 @@ void ModuleVisitor::EraseRenamedSymbol(const Symbol &useSymbol) {
   }
 }
 
-void ModuleVisitor::DoAddUse(const SourceName &location,
-    const SourceName &localName, Symbol &localSymbol, const Symbol &useSymbol) {
+void ModuleVisitor::DoAddUse(SourceName location, SourceName localName,
+    Symbol &localSymbol, const Symbol &useSymbol) {
   if (localName != useSymbol.name()) {
     EraseRenamedSymbol(useSymbol);
   }
-  localSymbol.attrs() = useSymbol.attrs() & ~Attrs{Attr::PUBLIC, Attr::PRIVATE};
-  localSymbol.flags() = useSymbol.flags();
+  if (auto *details{localSymbol.detailsIf<UseErrorDetails>()}) {
+    details->add_occurrence(location, *useModuleScope_);
+    return;
+  }
+
+  if (localSymbol.has<UnknownDetails>()) {
+    localSymbol.set_details(UseDetails{localName, useSymbol});
+    localSymbol.attrs() =
+        useSymbol.attrs() & ~Attrs{Attr::PUBLIC, Attr::PRIVATE};
+    localSymbol.flags() = useSymbol.flags();
+    return;
+  }
+
+  Symbol &localUltimate{localSymbol.GetUltimate()};
   const Symbol &useUltimate{useSymbol.GetUltimate()};
-  if (auto *useDetails{localSymbol.detailsIf<UseDetails>()}) {
-    const Symbol &localUltimate{localSymbol.GetUltimate()};
-    if (localUltimate.owner() == useUltimate.owner()) {
-      // use-associating the same symbol again -- ok
-    } else if (localUltimate.has<GenericDetails>() &&
-        useUltimate.has<GenericDetails>()) {
-      // use-associating generics with the same names: merge them into a
-      // new generic in this scope
-      auto generic1{localUltimate.get<GenericDetails>()};
-      AddGenericUse(generic1, localName, useUltimate);
-      generic1.AddUse(localSymbol);
-      // useSymbol has specific g and so does generic1
-      auto &generic2{useUltimate.get<GenericDetails>()};
-      if (generic1.derivedType() && generic2.derivedType() &&
-          generic1.derivedType() != generic2.derivedType()) {
+  if (&localUltimate == &useUltimate) {
+    // use-associating the same symbol again -- ok
+    return;
+  }
+
+  auto checkAmbiguousDerivedType{[this, location, localName](
+                                     const Symbol *t1, const Symbol *t2) {
+    if (!t1 || !t2) {
+      return true;
+    } else {
+      t1 = &t1->GetUltimate();
+      t2 = &t2->GetUltimate();
+      if (&t1 != &t2) {
         Say(location,
-            "Generic interface '%s' has ambiguous derived types"
-            " from modules '%s' and '%s'"_err_en_US,
-            localSymbol.name(), GetUsedModule(*useDetails).name(),
-            useUltimate.owner().GetName().value());
-        context().SetError(localSymbol);
+            "Generic interface '%s' has ambiguous derived types from modules '%s' and '%s'"_err_en_US,
+            localName, t1->owner().GetName().value(),
+            t2->owner().GetName().value());
+        return false;
+      }
+    }
+  }};
+
+  auto *localGeneric{localUltimate.detailsIf<GenericDetails>()};
+  const auto *useGeneric{useUltimate.detailsIf<GenericDetails>()};
+  auto combine{false};
+  if (localGeneric) {
+    if (useGeneric) {
+      if (!checkAmbiguousDerivedType(
+              localGeneric->derivedType(), useGeneric->derivedType())) {
+        return;
+      }
+      combine = true;
+    } else if (useUltimate.has<DerivedTypeDetails>()) {
+      if (checkAmbiguousDerivedType(
+              &useUltimate, localGeneric->derivedType())) {
+        combine = true;
       } else {
-        generic1.CopyFrom(generic2);
+        return;
       }
+    } else if (&useUltimate == &BypassGeneric(localUltimate)) {
+      return; // nothing to do; used subprogram is local's specific
+    }
+  } else if (useGeneric) {
+    if (localUltimate.has<DerivedTypeDetails>()) {
+      if (checkAmbiguousDerivedType(
+              &localUltimate, useGeneric->derivedType())) {
+        combine = true;
+      } else {
+        return;
+      }
+    } else if (&localUltimate == &BypassGeneric(useUltimate).GetUltimate()) {
+      // Local is the specific of the used generic; replace it.
       EraseSymbol(localSymbol);
-      MakeSymbol(localSymbol.name(), localSymbol.attrs(), std::move(generic1));
-    } else {
+      Symbol &newSymbol{MakeSymbol(localName,
+          useUltimate.attrs() & ~Attrs{Attr::PUBLIC, Attr::PRIVATE},
+          UseDetails{localName, useUltimate})};
+      newSymbol.flags() = useSymbol.flags();
+      return;
+    }
+  }
+  if (!combine) {
+    if (localSymbol.has<UseDetails>() || localSymbol.has<GenericDetails>()) {
       ConvertToUseError(localSymbol, location, *useModuleScope_);
+    } else {
+      Say(location,
+          "Cannot use-associate '%s'; it is already declared in this scope"_err_en_US,
+          localName)
+          .Attach(localSymbol.name(), "Previous declaration of '%s'"_en_US,
+              localName);
     }
-  } else if (auto *genericDetails{localSymbol.detailsIf<GenericDetails>()}) {
-    if (const auto *useDetails{useUltimate.detailsIf<GenericDetails>()}) {
-      AddGenericUse(*genericDetails, localName, useUltimate);
-      if (genericDetails->derivedType() && useDetails->derivedType() &&
-          genericDetails->derivedType() != useDetails->derivedType()) {
-        Say(location,
-            "Generic interface '%s' has ambiguous derived types"
-            " from modules '%s' and '%s'"_err_en_US,
-            localSymbol.name(),
-            genericDetails->derivedType()->owner().GetName().value(),
-            useDetails->derivedType()->owner().GetName().value());
-      } else {
-        genericDetails->CopyFrom(*useDetails);
-      }
+    return;
+  }
+
+  // Two items are being use-associated from 
diff erent modules
+  // to the same local name.  At least one of them must be a generic,
+  // and the other one can be a generic or a derived type.
+  // (It could also have been the specific of the generic, but those
+  // cases are handled above without needing to make a local copy of the
+  // generic.)
+
+  if (localGeneric) {
+    if (localSymbol.has<UseDetails>()) {
+      // Create a local copy of a previously use-associated generic so that
+      // it can be locally extended without corrupting the original.
+      GenericDetails generic;
+      generic.CopyFrom(*localGeneric);
+      EraseSymbol(localSymbol);
+      Symbol &newSymbol{MakeSymbol(
+          localSymbol.name(), localSymbol.attrs(), std::move(generic))};
+      newSymbol.flags() = localSymbol.flags();
+      localGeneric = &newSymbol.get<GenericDetails>();
+      localGeneric->AddUse(localSymbol);
+    }
+    if (useGeneric) {
+      // Combine two use-associated generics
+      localSymbol.attrs() =
+          useSymbol.attrs() & ~Attrs{Attr::PUBLIC, Attr::PRIVATE};
+      localSymbol.flags() = useSymbol.flags();
+      AddGenericUse(*localGeneric, localName, useUltimate);
+      localGeneric->CopyFrom(*useGeneric);
     } else {
-      ConvertToUseError(localSymbol, location, *useModuleScope_);
+      CHECK(useUltimate.has<DerivedTypeDetails>());
+      localGeneric->set_derivedType(
+          AddGenericUse(*localGeneric, localName, useUltimate));
     }
-  } else if (auto *details{localSymbol.detailsIf<UseErrorDetails>()}) {
-    details->add_occurrence(location, *useModuleScope_);
-  } else if (!localSymbol.has<UnknownDetails>()) {
-    Say(location,
-        "Cannot use-associate '%s'; it is already declared in this scope"_err_en_US,
-        localName)
-        .Attach(localSymbol.name(), "Previous declaration of '%s'"_en_US,
-            localName);
   } else {
-    localSymbol.set_details(UseDetails{localName, useSymbol});
+    CHECK(useGeneric && localUltimate.has<DerivedTypeDetails>());
+    CHECK(localSymbol.has<UseDetails>());
+    // Create a local copy of the use-associated generic, then extend it
+    // with the local derived type.
+    GenericDetails generic;
+    generic.CopyFrom(*useGeneric);
+    EraseSymbol(localSymbol);
+    Symbol &newSymbol{MakeSymbol(localName,
+        useUltimate.attrs() & ~Attrs{Attr::PUBLIC, Attr::PRIVATE},
+        std::move(generic))};
+    newSymbol.flags() = useUltimate.flags();
+    auto &newUseGeneric{newSymbol.get<GenericDetails>()};
+    AddGenericUse(newUseGeneric, localName, useUltimate);
+    newUseGeneric.AddUse(localSymbol);
+    newUseGeneric.set_derivedType(localSymbol);
   }
 }
 
@@ -2684,9 +2761,12 @@ void ModuleVisitor::AddUse(const GenericSpecInfo &info) {
 }
 
 // Create a UseDetails symbol for this USE and add it to generic
-void ModuleVisitor::AddGenericUse(
+Symbol &ModuleVisitor::AddGenericUse(
     GenericDetails &generic, const SourceName &name, const Symbol &useSymbol) {
-  generic.AddUse(currScope().MakeSymbol(name, {}, UseDetails{name, useSymbol}));
+  Symbol &newSymbol{
+      currScope().MakeSymbol(name, {}, UseDetails{name, useSymbol})};
+  generic.AddUse(newSymbol);
+  return newSymbol;
 }
 
 // Enforce C1406
@@ -5378,7 +5458,7 @@ std::optional<DerivedTypeSpec> DeclarationVisitor::ResolveDerivedType(
   symbol = &symbol->GetUltimate();
   if (auto *details{symbol->detailsIf<GenericDetails>()}) {
     if (details->derivedType()) {
-      symbol = details->derivedType();
+      symbol = &details->derivedType()->GetUltimate();
     }
   }
   if (symbol->has<DerivedTypeDetails>()) {

diff  --git a/flang/lib/Semantics/symbol.cpp b/flang/lib/Semantics/symbol.cpp
index ad80f901fb72c..cf24e28225c50 100644
--- a/flang/lib/Semantics/symbol.cpp
+++ b/flang/lib/Semantics/symbol.cpp
@@ -211,7 +211,8 @@ void GenericDetails::CopyFrom(const GenericDetails &from) {
   for (std::size_t i{0}; i < from.specificProcs_.size(); ++i) {
     if (std::find_if(specificProcs_.begin(), specificProcs_.end(),
             [&](const Symbol &mySymbol) {
-              return &mySymbol == &*from.specificProcs_[i];
+              return &mySymbol.GetUltimate() ==
+                  &from.specificProcs_[i]->GetUltimate();
             }) == specificProcs_.end()) {
       specificProcs_.push_back(from.specificProcs_[i]);
       bindingNames_.push_back(from.bindingNames_[i]);

diff  --git a/flang/test/Semantics/modfile07.f90 b/flang/test/Semantics/modfile07.f90
index 644c37e3d381d..48df7243e308c 100644
--- a/flang/test/Semantics/modfile07.f90
+++ b/flang/test/Semantics/modfile07.f90
@@ -399,8 +399,8 @@ subroutine test()
 end
 !Expect: m7c.mod
 !module m7c
-! use m7b, only: g => g_real
 ! use m7a, only: g => g_integer
+! use m7b, only: g => g_real
 ! interface g
 !  procedure :: s
 ! end interface
@@ -481,8 +481,8 @@ subroutine test()
 end
 !Expect: m8c.mod
 !module m8c
-! use m8b, only: g
 ! use m8a, only: g
+! use m8b, only: g
 ! interface g
 !  procedure :: s
 ! end interface
@@ -579,8 +579,8 @@ module m10c
 end
 !Expect: m10c.mod
 !module m10c
-! use m10b,only:operator(.ne.)
 ! use m10a,only:operator(.ne.)
+! use m10b,only:operator(.ne.)
 ! interface operator(.ne.)
 ! end interface
 !end
@@ -592,8 +592,8 @@ module m10d
 end
 !Expect: m10d.mod
 !module m10d
-! use m10c,only:operator(.ne.)
 ! use m10a,only:operator(.ne.)
+! use m10c,only:operator(.ne.)
 ! interface operator(.ne.)
 ! end interface
 ! private::operator(.ne.)

diff  --git a/flang/test/Semantics/resolve17.f90 b/flang/test/Semantics/resolve17.f90
index 96ab5cd11b461..b2c6d0e26eeea 100644
--- a/flang/test/Semantics/resolve17.f90
+++ b/flang/test/Semantics/resolve17.f90
@@ -265,3 +265,104 @@ module m12d
   interface g
   end interface
 end module
+
+module m13a
+ contains
+  subroutine subr
+  end subroutine
+end module
+module m13b
+  use m13a
+  interface subr
+    module procedure subr
+  end interface
+end module
+module m13c
+  use m13a
+  use m13b
+ contains
+  subroutine test
+    call subr
+  end subroutine
+end module
+module m13d
+  use m13b
+  use m13a
+ contains
+  subroutine test
+    call subr
+  end subroutine
+end module
+
+module m14a
+  type :: foo
+    integer :: n
+  end type
+end module
+module m14b
+  interface foo
+    module procedure bar
+  end interface
+ contains
+  real function bar(x)
+    real, intent(in) :: x
+    bar = x
+  end function
+end module
+module m14c
+  use m14a
+  use m14b
+  type(foo) :: x
+end module
+module m14d
+  use m14a
+  use m14b
+  type(foo) :: x
+ contains
+  subroutine test
+    real :: y
+    y = foo(1.0)
+    x = foo(2)
+  end subroutine
+end module
+module m14e
+  use m14b
+  use m14a
+  type(foo) :: x
+ contains
+  subroutine test
+    real :: y
+    y = foo(1.0)
+    x = foo(2)
+  end subroutine
+end module
+
+module m15a
+  interface foo
+    module procedure bar
+  end interface
+ contains
+  subroutine bar
+  end subroutine
+end module
+module m15b
+  !ERROR: Cannot use-associate 'foo'; it is already declared in this scope
+  use m15a
+ contains
+  subroutine foo
+  end subroutine
+end module
+module m15c
+ contains
+  subroutine foo
+  end subroutine
+end module
+module m15d
+  use m15a
+  use m15c
+ contains
+  subroutine test
+    !ERROR: Reference to 'foo' is ambiguous
+    call foo
+  end subroutine
+end module


        


More information about the flang-commits mailing list