From f1814c2329b25c3b12d0b1f281a2443428c05bb2 Mon Sep 17 00:00:00 2001 From: Peter Klausler Date: Fri, 30 Aug 2024 10:27:11 -0700 Subject: [PATCH] [flang] Fix spurious error with separate module procedures 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. --- flang/include/flang/Semantics/expression.h | 2 +- flang/include/flang/Semantics/semantics.h | 2 ++ flang/lib/Semantics/expression.cpp | 41 ++++++++++++++-------- flang/lib/Semantics/semantics.cpp | 6 ++-- flang/test/Semantics/smp-proc-ref.f90 | 20 +++++++++++ 5 files changed, 53 insertions(+), 18 deletions(-) create mode 100644 flang/test/Semantics/smp-proc-ref.f90 diff --git a/flang/include/flang/Semantics/expression.h b/flang/include/flang/Semantics/expression.h index a224b08da21da..b1304d704232d 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>; - bool ResolveForward(const Symbol &); + const Symbol *ResolveForward(const Symbol &); std::pair 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 ec8d12b0f9865..e73f9d2e85d58 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 4f8632a2055d9..60db02dc764b4 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()}) { @@ -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()) { + const Symbol *resolved{nullptr}; + if (auto iter{scope.find(symbol.name())}; iter != scope.cend()) { + resolved = &*iter->second; + } + if (!resolved || resolved->has()) { // 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()); @@ -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 ExpressionAnalyzer::ResolveGeneric( } if (const auto *details{ultimate.detailsIf()}) { 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 procedure{ characteristics::Procedure::Characterize( - ProcedureDesignator{specific}, context_.foldingContext(), + ProcedureDesignator{*specific}, context_.foldingContext(), /*emitError=*/false)}) { ActualArguments localActuals{actuals}; - if (specific.has()) { - if (!adjustActuals.value()(specific, localActuals)) { + if (specific->has()) { + if (!adjustActuals.value()(*specific, localActuals)) { continue; } } @@ -2751,9 +2757,9 @@ std::pair 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); + const Symbol &ultimate{symbol->GetUltimate()}; CheckForBadRecursion(name.source, ultimate); bool dueToAmbiguity{false}; bool isGenericInterface{ultimate.has()}; diff --git a/flang/lib/Semantics/semantics.cpp b/flang/lib/Semantics/semantics.cpp index f7a277d1b414f..8592d1e5d6217 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 0000000000000..9a2fae442e8e7 --- /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