[flang] Warn about undefined function results #99533
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
flang
|
@llvm/pr-subscribers-flang-semantics Author: Peter Klausler (klausler) ChangesWhen the result of a function never appears in a variable definition context, emit a warning. If the function has multiple result variables due to alternate ENTRY statements, any definition will suffice. The implementation of this check is tied to the general variable definability checking utility in semantics. Every variable definition context uses it to ensure that no undefinable variable is being defined. A set of defined variables is maintained in the SemanticsContext and, when the warning is enabled and no fatal error has been reported, the scope tree is traversed and all the function subprograms' results are tested for membership in that set. Full diff: https://github.com/llvm/llvm-project/pull/99533.diff 11 Files Affected:
diff --git a/flang/include/flang/Common/Fortran-features.h b/flang/include/flang/Common/Fortran-features.h
index 7346d702b073d..938da08e19d6b 100644
--- a/flang/include/flang/Common/Fortran-features.h
+++ b/flang/include/flang/Common/Fortran-features.h
@@ -70,7 +70,7 @@ ENUM_CLASS(UsageWarning, Portability, PointerToUndefinable,
IgnoredIntrinsicFunctionType, PreviousScalarUse,
RedeclaredInaccessibleComponent, ImplicitShared, IndexVarRedefinition,
IncompatibleImplicitInterfaces, BadTypeForTarget,
- VectorSubscriptFinalization)
+ VectorSubscriptFinalization, UndefinedFunctionResult)
using LanguageFeatures = EnumSet<LanguageFeature, LanguageFeature_enumSize>;
using UsageWarnings = EnumSet<UsageWarning, UsageWarning_enumSize>;
@@ -144,6 +144,7 @@ class LanguageFeatureControl {
warnUsage_.set(UsageWarning::IncompatibleImplicitInterfaces);
warnUsage_.set(UsageWarning::BadTypeForTarget);
warnUsage_.set(UsageWarning::VectorSubscriptFinalization);
+ warnUsage_.set(UsageWarning::UndefinedFunctionResult);
}
LanguageFeatureControl(const LanguageFeatureControl &) = default;
diff --git a/flang/include/flang/Semantics/semantics.h b/flang/include/flang/Semantics/semantics.h
index 3ee71fe485524..ec8d12b0f9865 100644
--- a/flang/include/flang/Semantics/semantics.h
+++ b/flang/include/flang/Semantics/semantics.h
@@ -254,6 +254,9 @@ class SemanticsContext {
// behavior.
CommonBlockList GetCommonBlocks() const;
+ void NoteDefinedSymbol(const Symbol &);
+ bool IsSymbolDefined(const Symbol &) const;
+
private:
struct ScopeIndexComparator {
bool operator()(parser::CharBlock, parser::CharBlock) const;
@@ -303,6 +306,7 @@ class SemanticsContext {
std::unique_ptr<CommonBlockMap> commonBlockMap_;
ModuleDependences moduleDependences_;
std::map<const Symbol *, SourceName> moduleFileOutputRenamings_;
+ UnorderedSymbolSet isDefined_;
};
class Semantics {
diff --git a/flang/include/flang/Semantics/tools.h b/flang/include/flang/Semantics/tools.h
index 0b5308d9242de..2654804307826 100644
--- a/flang/include/flang/Semantics/tools.h
+++ b/flang/include/flang/Semantics/tools.h
@@ -52,7 +52,6 @@ const Symbol *FindPointerComponent(const DeclTypeSpec &);
const Symbol *FindPointerComponent(const Symbol &);
const Symbol *FindInterface(const Symbol &);
const Symbol *FindSubprogram(const Symbol &);
-const Symbol *FindFunctionResult(const Symbol &);
const Symbol *FindOverriddenBinding(
const Symbol &, bool &isInaccessibleDeferred);
const Symbol *FindGlobal(const Symbol &);
diff --git a/flang/lib/Semantics/check-purity.cpp b/flang/lib/Semantics/check-purity.cpp
index 55a9a2f107388..1046f363e9485 100644
--- a/flang/lib/Semantics/check-purity.cpp
+++ b/flang/lib/Semantics/check-purity.cpp
@@ -31,7 +31,7 @@ void PurityChecker::Enter(const parser::FunctionSubprogram &func) {
stmt.source, std::get<std::list<parser::PrefixSpec>>(stmt.statement.t));
}
-void PurityChecker::Leave(const parser::FunctionSubprogram &) { Left(); }
+void PurityChecker::Leave(const parser::FunctionSubprogram &func) { Left(); }
bool PurityChecker::InPureSubprogram() const {
return pureDepth_ >= 0 && depth_ >= pureDepth_;
diff --git a/flang/lib/Semantics/definable.cpp b/flang/lib/Semantics/definable.cpp
index 96af46abd6180..7def2f5dbb398 100644
--- a/flang/lib/Semantics/definable.cpp
+++ b/flang/lib/Semantics/definable.cpp
@@ -127,6 +127,10 @@ static std::optional<parser::Message> WhyNotDefinableBase(parser::CharBlock at,
(!IsPointer(ultimate) || (isWholeSymbol && isPointerDefinition))) {
return BlameSymbol(
at, "'%s' is an INTENT(IN) dummy argument"_en_US, original);
+ } else if (acceptAllocatable) {
+ // allocating a function result doesn't count as a def'n
+ } else {
+ scope.context().NoteDefinedSymbol(ultimate);
}
if (const Scope * pure{FindPureProcedureContaining(scope)}) {
// Additional checking for pure subprograms.
diff --git a/flang/lib/Semantics/semantics.cpp b/flang/lib/Semantics/semantics.cpp
index c09734e5676f3..16f5b53b07be6 100644
--- a/flang/lib/Semantics/semantics.cpp
+++ b/flang/lib/Semantics/semantics.cpp
@@ -168,6 +168,27 @@ using StatementSemanticsPass2 = SemanticsVisitor<AllocateChecker,
ReturnStmtChecker, SelectRankConstructChecker, SelectTypeChecker,
StopChecker>;
+static void WarnUndefinedFunctionResult(
+ SemanticsContext &context, const Scope &scope) {
+ if (const Symbol * symbol{scope.symbol()}) {
+ if (const auto *subp{symbol->detailsIf<SubprogramDetails>()}) {
+ if (!subp->isInterface() && !subp->stmtFunction()) {
+ if (const Symbol * result{FindFunctionResult(*symbol)}) {
+ if (!context.IsSymbolDefined(*result)) {
+ context.Say(
+ symbol->name(), "Function result is never defined"_warn_en_US);
+ }
+ }
+ }
+ }
+ }
+ if (!scope.IsModuleFile()) {
+ for (const Scope &child : scope.children()) {
+ WarnUndefinedFunctionResult(context, child);
+ }
+ }
+}
+
static bool PerformStatementSemantics(
SemanticsContext &context, parser::Program &program) {
ResolveNames(context, program, context.globalScope());
@@ -187,6 +208,9 @@ static bool PerformStatementSemantics(
SemanticsVisitor<CUDAChecker>{context}.Walk(program);
}
if (!context.AnyFatalError()) {
+ if (context.ShouldWarn(common::UsageWarning::UndefinedFunctionResult)) {
+ WarnUndefinedFunctionResult(context, context.globalScope());
+ }
pass2.CompileDataInitializationsIntoInitializers();
}
return !context.AnyFatalError();
@@ -712,4 +736,19 @@ CommonBlockList SemanticsContext::GetCommonBlocks() const {
return {};
}
+void SemanticsContext::NoteDefinedSymbol(const Symbol &symbol) {
+ isDefined_.insert(symbol);
+ if (IsFunctionResult(symbol)) { // FUNCTION or ENTRY
+ if (const Symbol * func{symbol.owner().symbol()}) {
+ if (const Symbol * result{FindFunctionResult(*func)}) {
+ isDefined_.insert(*result);
+ }
+ }
+ }
+}
+
+bool SemanticsContext::IsSymbolDefined(const Symbol &symbol) const {
+ return isDefined_.find(symbol) != isDefined_.end();
+}
+
} // namespace Fortran::semantics
diff --git a/flang/test/Evaluate/folding08.f90 b/flang/test/Evaluate/folding08.f90
index 1b2e5605e85d4..53603474fe1c8 100644
--- a/flang/test/Evaluate/folding08.f90
+++ b/flang/test/Evaluate/folding08.f90
@@ -11,6 +11,11 @@ module m
end type
type(t) :: ta(0:2)
character(len=2) :: ca(-1:1)
+ interface
+ function foo()
+ real :: foo(2:3,4:6)
+ end function
+ end interface
integer, parameter :: lbtadim = lbound(ta,1)
logical, parameter :: test_lbtadim = lbtadim == 0
integer, parameter :: ubtadim = ubound(ta,1)
@@ -47,9 +52,6 @@ module m
logical, parameter :: test_lb_empty_dim = lbound(empty, 1) == 1
logical, parameter :: test_ub_empty_dim = ubound(empty, 1) == 0
contains
- function foo()
- real :: foo(2:3,4:6)
- end function
subroutine test(n1,a1,a2)
integer, intent(in) :: n1
real, intent(in) :: a1(1:n1), a2(0:*)
diff --git a/flang/test/Semantics/call02.f90 b/flang/test/Semantics/call02.f90
index bc3dd6075969c..0ec5530f98089 100644
--- a/flang/test/Semantics/call02.f90
+++ b/flang/test/Semantics/call02.f90
@@ -72,6 +72,7 @@ subroutine callme(f)
contains
elemental real function elem03(x)
real, value :: x
+ elem03 = 0.
end function
subroutine test
intrinsic :: cos
@@ -87,6 +88,7 @@ subroutine test
contains
elemental real function elem04(x)
real, value :: x
+ elem04 = 0.
end function
end subroutine
end module
diff --git a/flang/test/Semantics/call05.f90 b/flang/test/Semantics/call05.f90
index 8a4386e28e2bc..a06fe4f196c8c 100644
--- a/flang/test/Semantics/call05.f90
+++ b/flang/test/Semantics/call05.f90
@@ -155,11 +155,13 @@ subroutine smb(b)
function return_deferred_length_ptr()
character(len=:), pointer :: return_deferred_length_ptr
+ return_deferred_length_ptr => p2
end function
function return_explicit_length_ptr(n)
integer :: n
character(len=n), pointer :: return_explicit_length_ptr
+ return_explicit_length_ptr => p2(1:n)
end function
subroutine test()
diff --git a/flang/test/Semantics/contiguous01.f90 b/flang/test/Semantics/contiguous01.f90
index 0f086624a20ae..89382775261b8 100644
--- a/flang/test/Semantics/contiguous01.f90
+++ b/flang/test/Semantics/contiguous01.f90
@@ -30,8 +30,10 @@ function func(ashape,arank) result(r)
contiguous r2
!PORTABILITY: CONTIGUOUS entity 'e' should be an array pointer, assumed-shape, or assumed-rank
entry e() result(r2)
+ r2 = 0
end
function fp()
real, pointer, contiguous :: fp(:) ! ok
+ nullify(fp)
end
end
diff --git a/flang/test/Semantics/resolve53.f90 b/flang/test/Semantics/resolve53.f90
index 1b0f3f8f8e385..0ab4b7c4d303e 100644
--- a/flang/test/Semantics/resolve53.f90
+++ b/flang/test/Semantics/resolve53.f90
@@ -227,14 +227,17 @@ module m14
real function f1(x, y)
real, intent(in) :: x
logical, intent(in) :: y
+ f1 = 0.
end
integer function f2(x, y)
integer, intent(in) :: x
logical, intent(in) :: y
+ f2 = 0.
end
real function f3(x, y)
real, value :: x
logical, value :: y
+ f3 = 0.
end
end module
@@ -447,12 +450,15 @@ module m19
contains
integer function f1(i)
integer, intent(in) :: i
+ f1 = 0
end
integer function f2(i, j)
integer, value :: i, j
+ f2 = 0
end
integer function f3(i, j)
integer, intent(in) :: i, j
+ f3 = 0
end
end
|
|
Thanks for working on this. |
This patch contains many modified tests. |
jeanPerier
reviewed
Jul 19, 2024
There was a problem hiding this comment.
Definition contexts are not the only way to define the result. There are at least three cases I can think of where this warning will probably be emitted in bogus manner:
real function defined_in_call()
call init(defined_in_call)
end function
function defined_via_pointer()
real, pointer :: p
real, target :: defined_via_pointer
p => defined_via_pointer
p = 1
end function
subroutine init(x)
real :: x
x = 1
end subroutine
module test
type t
integer :: i = 1
end type
contains
type(t) function defined_via_init()
end function
end module
|
Thanks for those cases; will update. |
Right, but I couldn't find a test of the warning being emitted, just the opposite. |
When the result of a function never appears in a variable definition context, emit a warning. If the function has multiple result variables due to alternate ENTRY statements, any definition will suffice. The implementation of this check is tied to the general variable definability checking utility in semantics. Every variable definition context uses it to ensure that no undefinable variable is being defined. A set of defined variables is maintained in the SemanticsContext and, when the warning is enabled and no fatal error has been reported, the scope tree is traversed and all the function subprograms' results are tested for membership in that set.
|
Tests added, other cases considered. |
jeanPerier
approved these changes
Jul 22, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When the result of a function never appears in a variable definition context, emit a warning.
If the function has multiple result variables due to alternate ENTRY statements, any definition will suffice.
The implementation of this check is tied to the general variable definability checking utility in semantics. Every variable definition context uses it to ensure that no undefinable variable is being defined. A set of defined variables is maintained in the SemanticsContext and, when the warning is enabled and no fatal error has been reported, the scope tree is traversed and all the function subprograms' results are tested for membership in that set.