[flang-commits] [flang] [flang] Make proc characterization error conditional for generics (PR #89429)
Peter Klausler via flang-commits
flang-commits at lists.llvm.org
Fri Apr 19 11:10:03 PDT 2024
https://github.com/klausler created https://github.com/llvm/llvm-project/pull/89429
When the characteristics of a procedure depend on a procedure that hasn't yet been defined, the compiler currently emits an unconditional error message. This includes the case of a procedure whose characteristics depend, perhaps indirectly, on itself. However, in the case where the characteristics of a procedure are needed to resolve a generic, we should not emit an error for a hitherto undefined procedure -- either the call will resolve to another specific procedure, in which case the error is spurious, or it won't, and then an error will issue anyway.
Fixes https://github.com/llvm/llvm-project/issues/88677.
>From 614ccc88d99b7ca4edddbf48664669e3a4954e54 Mon Sep 17 00:00:00 2001
From: Peter Klausler <pklausler at nvidia.com>
Date: Fri, 19 Apr 2024 11:04:11 -0700
Subject: [PATCH] [flang] Make proc characterization error conditional for
generics
When the characteristics of a procedure depend on a procedure
that hasn't yet been defined, the compiler currently emits an
unconditional error message. This includes the case of a
procedure whose characteristics depend, perhaps indirectly, on
itself. However, in the case where the characteristics of a
procedure are needed to resolve a generic, we should not emit
an error for a hitherto undefined procedure -- either the call
will resolve to another specific procedure, in which case the
error is spurious, or it won't, and then an error will issue
anyway.
Fixes https://github.com/llvm/llvm-project/issues/88677.
---
.../include/flang/Evaluate/characteristics.h | 2 +-
flang/lib/Evaluate/characteristics.cpp | 75 +++++++++++--------
flang/lib/Evaluate/check-expression.cpp | 12 +--
flang/lib/Evaluate/tools.cpp | 4 +-
flang/lib/Lower/Bridge.cpp | 3 +-
flang/lib/Lower/CallInterface.cpp | 6 +-
flang/lib/Semantics/check-call.cpp | 4 +-
flang/lib/Semantics/expression.cpp | 5 +-
flang/lib/Semantics/pointer-assignment.cpp | 6 +-
flang/test/Semantics/resolve102.f90 | 13 ++++
10 files changed, 78 insertions(+), 52 deletions(-)
diff --git a/flang/include/flang/Evaluate/characteristics.h b/flang/include/flang/Evaluate/characteristics.h
index 82c31c0c404301..8aa065b025a4fa 100644
--- a/flang/include/flang/Evaluate/characteristics.h
+++ b/flang/include/flang/Evaluate/characteristics.h
@@ -365,7 +365,7 @@ struct Procedure {
static std::optional<Procedure> Characterize(
const semantics::Symbol &, FoldingContext &);
static std::optional<Procedure> Characterize(
- const ProcedureDesignator &, FoldingContext &);
+ const ProcedureDesignator &, FoldingContext &, bool emitError);
static std::optional<Procedure> Characterize(
const ProcedureRef &, FoldingContext &);
static std::optional<Procedure> Characterize(
diff --git a/flang/lib/Evaluate/characteristics.cpp b/flang/lib/Evaluate/characteristics.cpp
index 688a856220a117..ccbb19d9a324d6 100644
--- a/flang/lib/Evaluate/characteristics.cpp
+++ b/flang/lib/Evaluate/characteristics.cpp
@@ -576,11 +576,11 @@ static std::optional<DummyArgument> CharacterizeDummyArgument(
semantics::UnorderedSymbolSet seenProcs);
static std::optional<FunctionResult> CharacterizeFunctionResult(
const semantics::Symbol &symbol, FoldingContext &context,
- semantics::UnorderedSymbolSet seenProcs);
+ semantics::UnorderedSymbolSet seenProcs, bool emitError);
static std::optional<Procedure> CharacterizeProcedure(
const semantics::Symbol &original, FoldingContext &context,
- semantics::UnorderedSymbolSet seenProcs) {
+ semantics::UnorderedSymbolSet seenProcs, bool emitError) {
const auto &symbol{ResolveAssociations(original)};
if (seenProcs.find(symbol) != seenProcs.end()) {
std::string procsList{GetSeenProcs(seenProcs)};
@@ -591,6 +591,14 @@ static std::optional<Procedure> CharacterizeProcedure(
return std::nullopt;
}
seenProcs.insert(symbol);
+ auto CheckForNested{[&](const Symbol &symbol) {
+ if (emitError) {
+ CHECK(!getenv("PMK"));
+ context.messages().Say(
+ "Procedure '%s' is referenced before being sufficiently defined in a context where it must be so"_err_en_US,
+ symbol.name());
+ }
+ }};
auto result{common::visit(
common::visitors{
[&](const semantics::SubprogramDetails &subp)
@@ -598,7 +606,7 @@ static std::optional<Procedure> CharacterizeProcedure(
Procedure result;
if (subp.isFunction()) {
if (auto fr{CharacterizeFunctionResult(
- subp.result(), context, seenProcs)}) {
+ subp.result(), context, seenProcs, emitError)}) {
result.functionResult = std::move(fr);
} else {
return std::nullopt;
@@ -641,8 +649,8 @@ static std::optional<Procedure> CharacterizeProcedure(
}
if (const semantics::Symbol *
interfaceSymbol{proc.procInterface()}) {
- auto result{
- CharacterizeProcedure(*interfaceSymbol, context, seenProcs)};
+ auto result{CharacterizeProcedure(
+ *interfaceSymbol, context, seenProcs, /*emitError=*/false)};
if (result && (IsDummy(symbol) || IsPointer(symbol))) {
// Dummy procedures and procedure pointers may not be
// ELEMENTAL, but we do accept the use of elemental intrinsic
@@ -675,8 +683,8 @@ static std::optional<Procedure> CharacterizeProcedure(
}
},
[&](const semantics::ProcBindingDetails &binding) {
- if (auto result{CharacterizeProcedure(
- binding.symbol(), context, seenProcs)}) {
+ if (auto result{CharacterizeProcedure(binding.symbol(), context,
+ seenProcs, /*emitError=*/false)}) {
if (binding.symbol().attrs().test(semantics::Attr::INTRINSIC)) {
result->attrs.reset(Procedure::Attr::Elemental);
}
@@ -695,7 +703,8 @@ static std::optional<Procedure> CharacterizeProcedure(
}
},
[&](const semantics::UseDetails &use) {
- return CharacterizeProcedure(use.symbol(), context, seenProcs);
+ return CharacterizeProcedure(
+ use.symbol(), context, seenProcs, /*emitError=*/false);
},
[](const semantics::UseErrorDetails &) {
// Ambiguous use-association will be handled later during symbol
@@ -703,25 +712,23 @@ static std::optional<Procedure> CharacterizeProcedure(
return std::optional<Procedure>{};
},
[&](const semantics::HostAssocDetails &assoc) {
- return CharacterizeProcedure(assoc.symbol(), context, seenProcs);
+ return CharacterizeProcedure(
+ assoc.symbol(), context, seenProcs, /*emitError=*/false);
},
[&](const semantics::GenericDetails &generic) {
if (const semantics::Symbol * specific{generic.specific()}) {
- return CharacterizeProcedure(*specific, context, seenProcs);
+ return CharacterizeProcedure(
+ *specific, context, seenProcs, emitError);
} else {
return std::optional<Procedure>{};
}
},
[&](const semantics::EntityDetails &) {
- context.messages().Say(
- "Procedure '%s' is referenced before being sufficiently defined in a context where it must be so"_err_en_US,
- symbol.name());
+ CheckForNested(symbol);
return std::optional<Procedure>{};
},
[&](const semantics::SubprogramNameDetails &) {
- context.messages().Say(
- "Procedure '%s' is referenced before being sufficiently defined in a context where it must be so"_err_en_US,
- symbol.name());
+ CheckForNested(symbol);
return std::optional<Procedure>{};
},
[&](const auto &) {
@@ -752,7 +759,8 @@ static std::optional<Procedure> CharacterizeProcedure(
static std::optional<DummyProcedure> CharacterizeDummyProcedure(
const semantics::Symbol &symbol, FoldingContext &context,
semantics::UnorderedSymbolSet seenProcs) {
- if (auto procedure{CharacterizeProcedure(symbol, context, seenProcs)}) {
+ if (auto procedure{CharacterizeProcedure(
+ symbol, context, seenProcs, /*emitError=*/true)}) {
// Dummy procedures may not be elemental. Elemental dummy procedure
// interfaces are errors when the interface is not intrinsic, and that
// error is caught elsewhere. Elemental intrinsic interfaces are
@@ -854,7 +862,8 @@ std::optional<DummyArgument> DummyArgument::FromActual(std::string &&name,
std::move(name), std::move(obj));
},
[&](const ProcedureDesignator &designator) {
- if (auto proc{Procedure::Characterize(designator, context)}) {
+ if (auto proc{Procedure::Characterize(
+ designator, context, /*emitError=*/true)}) {
return std::make_optional<DummyArgument>(
std::move(name), DummyProcedure{std::move(*proc)});
} else {
@@ -988,7 +997,7 @@ bool FunctionResult::operator==(const FunctionResult &that) const {
static std::optional<FunctionResult> CharacterizeFunctionResult(
const semantics::Symbol &symbol, FoldingContext &context,
- semantics::UnorderedSymbolSet seenProcs) {
+ semantics::UnorderedSymbolSet seenProcs, bool emitError) {
if (const auto *object{symbol.detailsIf<semantics::ObjectEntityDetails>()}) {
if (auto type{TypeAndShape::Characterize(
symbol, context, /*invariantOnly=*/false)}) {
@@ -1002,8 +1011,8 @@ static std::optional<FunctionResult> CharacterizeFunctionResult(
result.cudaDataAttr = object->cudaDataAttr();
return result;
}
- } else if (auto maybeProc{
- CharacterizeProcedure(symbol, context, seenProcs)}) {
+ } else if (auto maybeProc{CharacterizeProcedure(
+ symbol, context, seenProcs, emitError)}) {
FunctionResult result{std::move(*maybeProc)};
result.attrs.set(FunctionResult::Attr::Pointer);
return result;
@@ -1014,7 +1023,8 @@ static std::optional<FunctionResult> CharacterizeFunctionResult(
std::optional<FunctionResult> FunctionResult::Characterize(
const Symbol &symbol, FoldingContext &context) {
semantics::UnorderedSymbolSet seenProcs;
- return CharacterizeFunctionResult(symbol, context, seenProcs);
+ return CharacterizeFunctionResult(
+ symbol, context, seenProcs, /*emitError=*/false);
}
bool FunctionResult::IsAssumedLengthCharacter() const {
@@ -1360,27 +1370,26 @@ bool Procedure::CanOverride(
}
std::optional<Procedure> Procedure::Characterize(
- const semantics::Symbol &original, FoldingContext &context) {
+ const semantics::Symbol &symbol, FoldingContext &context) {
semantics::UnorderedSymbolSet seenProcs;
- return CharacterizeProcedure(original, context, seenProcs);
+ return CharacterizeProcedure(symbol, context, seenProcs, /*emitError=*/true);
}
std::optional<Procedure> Procedure::Characterize(
- const ProcedureDesignator &proc, FoldingContext &context) {
+ const ProcedureDesignator &proc, FoldingContext &context, bool emitError) {
if (const auto *symbol{proc.GetSymbol()}) {
- if (auto result{
- characteristics::Procedure::Characterize(*symbol, context)}) {
- return result;
- }
+ semantics::UnorderedSymbolSet seenProcs;
+ return CharacterizeProcedure(*symbol, context, seenProcs, emitError);
} else if (const auto *intrinsic{proc.GetSpecificIntrinsic()}) {
return intrinsic->characteristics.value();
+ } else {
+ return std::nullopt;
}
- return std::nullopt;
}
std::optional<Procedure> Procedure::Characterize(
const ProcedureRef &ref, FoldingContext &context) {
- if (auto callee{Characterize(ref.proc(), context)}) {
+ if (auto callee{Characterize(ref.proc(), context, /*emitError=*/true)}) {
if (callee->functionResult) {
if (const Procedure *
proc{callee->functionResult->IsProcedurePointer()}) {
@@ -1397,7 +1406,7 @@ std::optional<Procedure> Procedure::Characterize(
return Characterize(*procRef, context);
} else if (const auto *procDesignator{
std::get_if<ProcedureDesignator>(&expr.u)}) {
- return Characterize(*procDesignator, context);
+ return Characterize(*procDesignator, context, /*emitError=*/true);
} else if (const Symbol * symbol{UnwrapWholeSymbolOrComponentDataRef(expr)}) {
return Characterize(*symbol, context);
} else {
@@ -1409,7 +1418,7 @@ std::optional<Procedure> Procedure::Characterize(
std::optional<Procedure> Procedure::FromActuals(const ProcedureDesignator &proc,
const ActualArguments &args, FoldingContext &context) {
- auto callee{Characterize(proc, context)};
+ auto callee{Characterize(proc, context, /*emitError=*/true)};
if (callee) {
if (callee->dummyArguments.empty() &&
callee->attrs.test(Procedure::Attr::ImplicitInterface)) {
diff --git a/flang/lib/Evaluate/check-expression.cpp b/flang/lib/Evaluate/check-expression.cpp
index 0e14aa0957294c..7e42db7b6ed7ab 100644
--- a/flang/lib/Evaluate/check-expression.cpp
+++ b/flang/lib/Evaluate/check-expression.cpp
@@ -666,8 +666,8 @@ class CheckSpecificationExprHelper
"' not allowed for derived type components or type parameter"
" values";
}
- if (auto procChars{
- characteristics::Procedure::Characterize(x.proc(), context_)}) {
+ if (auto procChars{characteristics::Procedure::Characterize(
+ x.proc(), context_, /*emitError=*/true)}) {
const auto iter{std::find_if(procChars->dummyArguments.begin(),
procChars->dummyArguments.end(),
[](const characteristics::DummyArgument &dummy) {
@@ -856,8 +856,8 @@ class IsContiguousHelper
Result operator()(const Substring &) const { return std::nullopt; }
Result operator()(const ProcedureRef &x) const {
- if (auto chars{
- characteristics::Procedure::Characterize(x.proc(), context_)}) {
+ if (auto chars{characteristics::Procedure::Characterize(
+ x.proc(), context_, /*emitError=*/true)}) {
if (chars->functionResult) {
const auto &result{*chars->functionResult};
if (!result.IsProcedurePointer()) {
@@ -1103,8 +1103,8 @@ class StmtFunctionChecker
}
}
}
- if (auto chars{
- characteristics::Procedure::Characterize(proc, context_)}) {
+ if (auto chars{characteristics::Procedure::Characterize(
+ proc, context_, /*emitError=*/true)}) {
if (!chars->CanBeCalledViaImplicitInterface()) {
if (severity_) {
auto msg{
diff --git a/flang/lib/Evaluate/tools.cpp b/flang/lib/Evaluate/tools.cpp
index f514a25b010241..9a5f9130632ee8 100644
--- a/flang/lib/Evaluate/tools.cpp
+++ b/flang/lib/Evaluate/tools.cpp
@@ -1056,8 +1056,8 @@ class FindImpureCallHelper
explicit FindImpureCallHelper(FoldingContext &c) : Base{*this}, context_{c} {}
using Base::operator();
Result operator()(const ProcedureRef &call) const {
- if (auto chars{
- characteristics::Procedure::Characterize(call.proc(), context_)}) {
+ if (auto chars{characteristics::Procedure::Characterize(
+ call.proc(), context_, /*emitError=*/false)}) {
if (chars->attrs.test(characteristics::Procedure::Attr::Pure)) {
return (*this)(call.arguments());
}
diff --git a/flang/lib/Lower/Bridge.cpp b/flang/lib/Lower/Bridge.cpp
index 47bd6ace4e4b56..8b62fe8c022f80 100644
--- a/flang/lib/Lower/Bridge.cpp
+++ b/flang/lib/Lower/Bridge.cpp
@@ -3700,7 +3700,8 @@ class FirConverter : public Fortran::lower::AbstractConverter {
using DummyAttr = Fortran::evaluate::characteristics::DummyDataObject::Attr;
if (auto procedure =
Fortran::evaluate::characteristics::Procedure::Characterize(
- userDefinedAssignment.proc(), getFoldingContext()))
+ userDefinedAssignment.proc(), getFoldingContext(),
+ /*emitError=*/false))
if (!procedure->dummyArguments.empty())
if (const auto *dataArg = std::get_if<
Fortran::evaluate::characteristics::DummyDataObject>(
diff --git a/flang/lib/Lower/CallInterface.cpp b/flang/lib/Lower/CallInterface.cpp
index 29cdb3cff589ba..af0dd2aab91ee3 100644
--- a/flang/lib/Lower/CallInterface.cpp
+++ b/flang/lib/Lower/CallInterface.cpp
@@ -218,7 +218,7 @@ Fortran::lower::CallerInterface::characterize() const {
converter.getFoldingContext();
std::optional<Fortran::evaluate::characteristics::Procedure> characteristic =
Fortran::evaluate::characteristics::Procedure::Characterize(
- procRef.proc(), foldingContext);
+ procRef.proc(), foldingContext, /*emitError=*/false);
assert(characteristic && "Failed to get characteristic from procRef");
// The characteristic may not contain the argument characteristic if the
// ProcedureDesignator has no interface, or may mismatch in case of implicit
@@ -1543,7 +1543,7 @@ class SignatureBuilder
Fortran::lower::AbstractConverter &c)
: CallInterface{c}, procDesignator{&procDes},
proc{Fortran::evaluate::characteristics::Procedure::Characterize(
- procDes, converter.getFoldingContext())
+ procDes, converter.getFoldingContext(), /*emitError=*/false)
.value()} {}
/// Does the procedure characteristics being translated have alternate
/// returns ?
@@ -1672,7 +1672,7 @@ bool Fortran::lower::mustPassLengthWithDummyProcedure(
Fortran::lower::AbstractConverter &converter) {
std::optional<Fortran::evaluate::characteristics::Procedure> characteristics =
Fortran::evaluate::characteristics::Procedure::Characterize(
- procedure, converter.getFoldingContext());
+ procedure, converter.getFoldingContext(), /*emitError=*/false);
return ::mustPassLengthWithDummyProcedure(characteristics);
}
diff --git a/flang/lib/Semantics/check-call.cpp b/flang/lib/Semantics/check-call.cpp
index bd2f755855172a..6cbc3565dc3775 100644
--- a/flang/lib/Semantics/check-call.cpp
+++ b/flang/lib/Semantics/check-call.cpp
@@ -1597,8 +1597,8 @@ static void CheckReduce(
if (const auto *expr{operation->UnwrapExpr()}) {
if (const auto *designator{
std::get_if<evaluate::ProcedureDesignator>(&expr->u)}) {
- procChars =
- characteristics::Procedure::Characterize(*designator, context);
+ procChars = characteristics::Procedure::Characterize(
+ *designator, context, /*emitError=*/true);
} else if (const auto *ref{
std::get_if<evaluate::ProcedureRef>(&expr->u)}) {
procChars = characteristics::Procedure::Characterize(*ref, context);
diff --git a/flang/lib/Semantics/expression.cpp b/flang/lib/Semantics/expression.cpp
index 6af86de9dd81cb..a270e4b385e8db 100644
--- a/flang/lib/Semantics/expression.cpp
+++ b/flang/lib/Semantics/expression.cpp
@@ -2562,7 +2562,8 @@ std::pair<const Symbol *, bool> ExpressionAnalyzer::ResolveGeneric(
}
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)) {
@@ -3164,7 +3165,7 @@ std::optional<characteristics::Procedure> ExpressionAnalyzer::CheckCall(
}
if (!chars) {
chars = characteristics::Procedure::Characterize(
- proc, context_.foldingContext());
+ proc, context_.foldingContext(), /*emitError=*/true);
}
bool ok{true};
if (chars) {
diff --git a/flang/lib/Semantics/pointer-assignment.cpp b/flang/lib/Semantics/pointer-assignment.cpp
index 4b4ce153084d8e..60a496a63cb380 100644
--- a/flang/lib/Semantics/pointer-assignment.cpp
+++ b/flang/lib/Semantics/pointer-assignment.cpp
@@ -244,7 +244,8 @@ bool PointerAssignmentChecker::Check(const evaluate::FunctionRef<T> &f) {
} else if (const auto *intrinsic{f.proc().GetSpecificIntrinsic()}) {
funcName = intrinsic->name;
}
- auto proc{Procedure::Characterize(f.proc(), foldingContext_)};
+ auto proc{
+ Procedure::Characterize(f.proc(), foldingContext_, /*emitError=*/true)};
if (!proc) {
return false;
}
@@ -393,7 +394,8 @@ bool PointerAssignmentChecker::Check(const evaluate::ProcedureDesignator &d) {
symbol->name());
}
}
- if (auto chars{Procedure::Characterize(d, foldingContext_)}) {
+ if (auto chars{
+ Procedure::Characterize(d, foldingContext_, /*emitError=*/true)}) {
// Disregard the elemental attribute of RHS intrinsics.
if (symbol && symbol->GetUltimate().attrs().test(Attr::INTRINSIC)) {
chars->attrs.reset(Procedure::Attr::Elemental);
diff --git a/flang/test/Semantics/resolve102.f90 b/flang/test/Semantics/resolve102.f90
index 11f2ce9c8ea561..8f6e2246a57e79 100644
--- a/flang/test/Semantics/resolve102.f90
+++ b/flang/test/Semantics/resolve102.f90
@@ -106,3 +106,16 @@ pure integer function g(n)
g = size(arr)
end function
end
+
+module genericInSpec
+ interface int
+ procedure ifunc
+ end interface
+ contains
+ function ifunc(x)
+ integer a(int(kind(1))) ! generic is ok with most compilers
+ integer(size(a)), intent(in) :: x
+ ifunc = x
+ end
+end
+
More information about the flang-commits
mailing list