[flang-commits] [flang] d1e4a2d - [flang] Fix spurious error with separate module procedures (#106768)

via flang-commits flang-commits at lists.llvm.org
Wed Sep 4 10:54:50 PDT 2024


Author: Peter Klausler
Date: 2024-09-04T10:54:46-07:00
New Revision: d1e4a2d300f7c0c6b681ddf719132c81d348aaab

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

LOG: [flang] Fix spurious error with separate module procedures (#106768)

When the implementation of one SMP apparently references another in what
might be a specification expression, semantics may need to resolve it as
a forward reference, and to allow for the replacement of a
SubprogramNameDetails place-holding symbol with the final
SubprogramDetails symbol. Otherwise, as in the bug report below,
confusing error messages may result.

(The reference in question isn't really in the specification part of a
subprogram, but due to the syntactic ambiguity between the array element
assignment statement and a statement function definition, it appears to
be so at the time that the reference is processed.)

I needed to make DumpSymbols() available via SemanticsContext to analyze
this bug, and left that new API in place to make things easier next
time.

Fixes https://github.com/llvm/llvm-project/issues/106705.

Added: 
    flang/test/Semantics/smp-proc-ref.f90

Modified: 
    flang/include/flang/Semantics/expression.h
    flang/include/flang/Semantics/semantics.h
    flang/lib/Semantics/expression.cpp
    flang/lib/Semantics/semantics.cpp

Removed: 
    


################################################################################
diff  --git a/flang/include/flang/Semantics/expression.h b/flang/include/flang/Semantics/expression.h
index a224b08da21da7..b1304d704232dc 100644
--- a/flang/include/flang/Semantics/expression.h
+++ b/flang/include/flang/Semantics/expression.h
@@ -354,7 +354,7 @@ class ExpressionAnalyzer {
       parser::CharBlock, const ProcedureDesignator &, ActualArguments &);
   using AdjustActuals =
       std::optional<std::function<bool(const Symbol &, ActualArguments &)>>;
-  bool ResolveForward(const Symbol &);
+  const Symbol *ResolveForward(const Symbol &);
   std::pair<const Symbol *, bool /* failure due ambiguity */> ResolveGeneric(
       const Symbol &, const ActualArguments &, const AdjustActuals &,
       bool isSubroutine, bool mightBeStructureConstructor = false);

diff  --git a/flang/include/flang/Semantics/semantics.h b/flang/include/flang/Semantics/semantics.h
index ec8d12b0f98653..e73f9d2e85d589 100644
--- a/flang/include/flang/Semantics/semantics.h
+++ b/flang/include/flang/Semantics/semantics.h
@@ -257,6 +257,8 @@ class SemanticsContext {
   void NoteDefinedSymbol(const Symbol &);
   bool IsSymbolDefined(const Symbol &) const;
 
+  void DumpSymbols(llvm::raw_ostream &);
+
 private:
   struct ScopeIndexComparator {
     bool operator()(parser::CharBlock, parser::CharBlock) const;

diff  --git a/flang/lib/Semantics/expression.cpp b/flang/lib/Semantics/expression.cpp
index 4f8632a2055d99..60db02dc764b46 100644
--- a/flang/lib/Semantics/expression.cpp
+++ b/flang/lib/Semantics/expression.cpp
@@ -2650,9 +2650,9 @@ static int ComputeCudaMatchingDistance(
 // Handles a forward reference to a module function from what must
 // be a specification expression.  Return false if the symbol is
 // an invalid forward reference.
-bool ExpressionAnalyzer::ResolveForward(const Symbol &symbol) {
+const Symbol *ExpressionAnalyzer::ResolveForward(const Symbol &symbol) {
   if (context_.HasError(symbol)) {
-    return false;
+    return nullptr;
   }
   if (const auto *details{
           symbol.detailsIf<semantics::SubprogramNameDetails>()}) {
@@ -2661,8 +2661,13 @@ bool ExpressionAnalyzer::ResolveForward(const Symbol &symbol) {
       // checking a specification expression in a sibling module
       // procedure.  Resolve its names now so that its interface
       // is known.
+      const semantics::Scope &scope{symbol.owner()};
       semantics::ResolveSpecificationParts(context_, symbol);
-      if (symbol.has<semantics::SubprogramNameDetails>()) {
+      const Symbol *resolved{nullptr};
+      if (auto iter{scope.find(symbol.name())}; iter != scope.cend()) {
+        resolved = &*iter->second;
+      }
+      if (!resolved || resolved->has<semantics::SubprogramNameDetails>()) {
         // When the symbol hasn't had its details updated, we must have
         // already been in the process of resolving the function's
         // specification part; but recursive function calls are not
@@ -2670,8 +2675,8 @@ bool ExpressionAnalyzer::ResolveForward(const Symbol &symbol) {
         Say("The module function '%s' may not be referenced recursively in a specification expression"_err_en_US,
             symbol.name());
         context_.SetError(symbol);
-        return false;
       }
+      return resolved;
     } else if (inStmtFunctionDefinition_) {
       semantics::ResolveSpecificationParts(context_, symbol);
       CHECK(symbol.has<semantics::SubprogramDetails>());
@@ -2679,10 +2684,10 @@ bool ExpressionAnalyzer::ResolveForward(const Symbol &symbol) {
       Say("The internal function '%s' may not be referenced in a specification expression"_err_en_US,
           symbol.name());
       context_.SetError(symbol);
-      return false;
+      return nullptr;
     }
   }
-  return true;
+  return &symbol;
 }
 
 // Resolve a call to a generic procedure with given actual arguments.
@@ -2709,20 +2714,21 @@ std::pair<const Symbol *, bool> ExpressionAnalyzer::ResolveGeneric(
   }
   if (const auto *details{ultimate.detailsIf<semantics::GenericDetails>()}) {
     for (const Symbol &specific0 : details->specificProcs()) {
-      const Symbol &specific{BypassGeneric(specific0)};
-      if (isSubroutine != !IsFunction(specific)) {
+      const Symbol &specific1{BypassGeneric(specific0)};
+      if (isSubroutine != !IsFunction(specific1)) {
         continue;
       }
-      if (!ResolveForward(specific)) {
+      const Symbol *specific{ResolveForward(specific1)};
+      if (!specific) {
         continue;
       }
       if (std::optional<characteristics::Procedure> procedure{
               characteristics::Procedure::Characterize(
-                  ProcedureDesignator{specific}, context_.foldingContext(),
+                  ProcedureDesignator{*specific}, context_.foldingContext(),
                   /*emitError=*/false)}) {
         ActualArguments localActuals{actuals};
-        if (specific.has<semantics::ProcBindingDetails>()) {
-          if (!adjustActuals.value()(specific, localActuals)) {
+        if (specific->has<semantics::ProcBindingDetails>()) {
+          if (!adjustActuals.value()(*specific, localActuals)) {
             continue;
           }
         }
@@ -2751,9 +2757,9 @@ std::pair<const Symbol *, bool> ExpressionAnalyzer::ResolveGeneric(
           }
           if (!procedure->IsElemental()) {
             // takes priority over elemental match
-            nonElemental = &specific;
+            nonElemental = specific;
           } else {
-            elemental = &specific;
+            elemental = specific;
           }
           crtMatchingDistance = ComputeCudaMatchingDistance(
               context_.languageFeatures(), *procedure, localActuals);
@@ -2866,7 +2872,12 @@ auto ExpressionAnalyzer::GetCalleeAndArguments(const parser::Name &name,
   if (context_.HasError(symbol)) {
     return std::nullopt; // also handles null symbol
   }
-  const Symbol &ultimate{DEREF(symbol).GetUltimate()};
+  symbol = ResolveForward(*symbol);
+  if (!symbol) {
+    return std::nullopt;
+  }
+  name.symbol = const_cast<Symbol *>(symbol);
+  const Symbol &ultimate{symbol->GetUltimate()};
   CheckForBadRecursion(name.source, ultimate);
   bool dueToAmbiguity{false};
   bool isGenericInterface{ultimate.has<semantics::GenericDetails>()};

diff  --git a/flang/lib/Semantics/semantics.cpp b/flang/lib/Semantics/semantics.cpp
index f7a277d1b414f6..8592d1e5d6217e 100644
--- a/flang/lib/Semantics/semantics.cpp
+++ b/flang/lib/Semantics/semantics.cpp
@@ -655,10 +655,12 @@ void Semantics::EmitMessages(llvm::raw_ostream &os) {
   context_.messages().Emit(os, context_.allCookedSources());
 }
 
-void Semantics::DumpSymbols(llvm::raw_ostream &os) {
-  DoDumpSymbols(os, context_.globalScope());
+void SemanticsContext::DumpSymbols(llvm::raw_ostream &os) {
+  DoDumpSymbols(os, globalScope());
 }
 
+void Semantics::DumpSymbols(llvm::raw_ostream &os) { context_.DumpSymbols(os); }
+
 void Semantics::DumpSymbolsSources(llvm::raw_ostream &os) const {
   NameToSymbolMap symbols;
   GetSymbolNames(context_.globalScope(), symbols);

diff  --git a/flang/test/Semantics/smp-proc-ref.f90 b/flang/test/Semantics/smp-proc-ref.f90
new file mode 100644
index 00000000000000..9a2fae442e8e70
--- /dev/null
+++ b/flang/test/Semantics/smp-proc-ref.f90
@@ -0,0 +1,20 @@
+!RUN: %flang_fc1 -fsyntax-only %s
+module m
+  real :: qux(10)
+  interface
+    module subroutine bar(i)
+    end
+    module function baz()
+    end
+  end interface
+end
+
+submodule(m) sm
+ contains
+  module procedure bar
+    qux(i) = baz() ! ensure no bogus error here
+  end
+  module procedure baz
+    baz = 1.
+  end
+end


        


More information about the flang-commits mailing list