-
Notifications
You must be signed in to change notification settings - Fork 13.3k
[flang] Refine handling of NULL() actual to non-optional allocatable … #116126
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@llvm/pr-subscribers-flang-semantics Author: Peter Klausler (klausler) Changes…dummy We presently allow a NULL() actual argument to associate with a non-optional dummy allocatable argument only under INTENT(IN). This is too strict, as it precludes the case of a dummy argument with default intent. Continue to require that the actual argument be definable under INTENT(OUT) and INTENT(IN OUT), and (contra XLF) interpret NULL() as being an expression, not a definable variable, even when it is given an allocatable MOLD. Fixes #115984. Full diff: https://github.com/llvm/llvm-project/pull/116126.diff 4 Files Affected:
diff --git a/flang/include/flang/Common/Fortran-features.h b/flang/include/flang/Common/Fortran-features.h
index 74edbe44fdbb1c..78ba3f0d330292 100644
--- a/flang/include/flang/Common/Fortran-features.h
+++ b/flang/include/flang/Common/Fortran-features.h
@@ -38,7 +38,7 @@ ENUM_CLASS(LanguageFeature, BackslashEscapes, OldDebugLines,
DistinguishableSpecifics, DefaultSave, PointerInSeqType, NonCharacterFormat,
SaveMainProgram, SaveBigMainProgramVariables,
DistinctArrayConstructorLengths, PPCVector, RelaxedIntentInChecking,
- ForwardRefImplicitNoneData, NullActualForAllocatable,
+ NullActualForAllocatable, ForwardRefImplicitNoneData,
ActualIntegerConvertedToSmallerKind, HollerithOrCharacterAsBOZ,
BindingAsProcedure, StatementFunctionExtensions,
UseGenericIntrinsicWhenSpecificDoesntMatch, DataStmtExtensions,
@@ -72,7 +72,7 @@ ENUM_CLASS(UsageWarning, Portability, PointerToUndefinable,
PreviousScalarUse, RedeclaredInaccessibleComponent, ImplicitShared,
IndexVarRedefinition, IncompatibleImplicitInterfaces, BadTypeForTarget,
VectorSubscriptFinalization, UndefinedFunctionResult, UselessIomsg,
- MismatchingDummyProcedure)
+ MismatchingDummyProcedure, NullActualForDefaultIntentAllocatable)
using LanguageFeatures = EnumSet<LanguageFeature, LanguageFeature_enumSize>;
using UsageWarnings = EnumSet<UsageWarning, UsageWarning_enumSize>;
diff --git a/flang/lib/Common/Fortran-features.cpp b/flang/lib/Common/Fortran-features.cpp
index fff796e42552a5..719ef2b0291ff8 100644
--- a/flang/lib/Common/Fortran-features.cpp
+++ b/flang/lib/Common/Fortran-features.cpp
@@ -80,8 +80,10 @@ LanguageFeatureControl::LanguageFeatureControl() {
warnUsage_.set(UsageWarning::VectorSubscriptFinalization);
warnUsage_.set(UsageWarning::UndefinedFunctionResult);
warnUsage_.set(UsageWarning::UselessIomsg);
+ warnUsage_.set(UsageWarning::NullActualForDefaultIntentAllocatable);
// New warnings, on by default
warnLanguage_.set(LanguageFeature::SavedLocalInSpecExpr);
+ warnLanguage_.set(LanguageFeature::NullActualForAllocatable);
}
// Ignore case and any inserted punctuation (like '-'/'_')
diff --git a/flang/lib/Semantics/check-call.cpp b/flang/lib/Semantics/check-call.cpp
index a161d2bdf9dbb8..59bbd8b7da56a7 100644
--- a/flang/lib/Semantics/check-call.cpp
+++ b/flang/lib/Semantics/check-call.cpp
@@ -766,21 +766,21 @@ static void CheckExplicitDataArg(const characteristics::DummyDataObject &dummy,
}
} else if (actualIsNull) {
if (dummyIsOptional) {
- } else if (dummy.intent == common::Intent::In) {
- // Extension (Intel, NAG, XLF): a NULL() pointer is an acceptable
- // actual argument for an INTENT(IN) allocatable dummy, and it
- // is treated as an unassociated allocatable.
- if (context.ShouldWarn(
- common::LanguageFeature::NullActualForAllocatable)) {
- messages.Say(common::LanguageFeature::NullActualForAllocatable,
- "Allocatable %s is associated with a null pointer"_port_en_US,
- dummyName);
- }
- } else {
+ } else if (dummy.intent == common::Intent::Default &&
+ context.ShouldWarn(
+ common::UsageWarning::NullActualForDefaultIntentAllocatable)) {
messages.Say(
- "A null pointer may not be associated with allocatable %s without INTENT(IN)"_err_en_US,
+ "A null pointer should not be associated with allocatable %s without INTENT(IN)"_warn_en_US,
+ dummyName);
+ } else if (dummy.intent == common::Intent::In &&
+ context.ShouldWarn(
+ common::LanguageFeature::NullActualForAllocatable)) {
+ messages.Say(common::LanguageFeature::NullActualForAllocatable,
+ "Allocatable %s is associated with a null pointer"_port_en_US,
dummyName);
}
+ // INTENT(OUT) and INTENT(IN OUT) cases are caught elsewhere as being
+ // undefinable actual arguments.
} else {
messages.Say(
"ALLOCATABLE %s must be associated with an ALLOCATABLE actual argument"_err_en_US,
@@ -1265,19 +1265,24 @@ static void CheckExplicitInterfaceArg(evaluate::ActualArgument &arg,
} else if (object.attrs.test(characteristics::DummyDataObject::
Attr::Allocatable) &&
evaluate::IsNullPointer(*expr)) {
- if (object.intent == common::Intent::In) {
- // Extension (Intel, NAG, XLF); see CheckExplicitDataArg.
- if (context.ShouldWarn(common::LanguageFeature::
- NullActualForAllocatable)) {
- messages.Say(
- common::LanguageFeature::NullActualForAllocatable,
- "Allocatable %s is associated with NULL()"_port_en_US,
- dummyName);
- }
- } else {
+ if (object.intent == common::Intent::Out ||
+ object.intent == common::Intent::InOut) {
messages.Say(
- "NULL() actual argument '%s' may not be associated with allocatable %s without INTENT(IN)"_err_en_US,
+ "NULL() actual argument '%s' may not be associated with allocatable dummy argument %s that is INTENT(OUT) or INTENT(IN OUT)"_err_en_US,
expr->AsFortran(), dummyName);
+ } else if (object.intent == common::Intent::Default &&
+ context.ShouldWarn(common::UsageWarning::
+ NullActualForDefaultIntentAllocatable)) {
+ messages.Say(common::UsageWarning::
+ NullActualForDefaultIntentAllocatable,
+ "NULL() actual argument '%s' should not be associated with allocatable dummy argument %s without INTENT(IN)"_warn_en_US,
+ expr->AsFortran(), dummyName);
+ } else if (context.ShouldWarn(common::LanguageFeature::
+ NullActualForAllocatable)) {
+ messages.Say(
+ common::LanguageFeature::NullActualForAllocatable,
+ "Allocatable %s is associated with %s"_port_en_US,
+ dummyName, expr->AsFortran());
}
} else {
messages.Say(
diff --git a/flang/test/Semantics/call27.f90 b/flang/test/Semantics/call27.f90
index 062df6e45da890..135d6c06dcb4ac 100644
--- a/flang/test/Semantics/call27.f90
+++ b/flang/test/Semantics/call27.f90
@@ -1,12 +1,26 @@
! RUN: %python %S/test_errors.py %s %flang_fc1 -pedantic
! Catch NULL() actual argument association with allocatable dummy argument
program test
- !ERROR: NULL() actual argument 'NULL()' may not be associated with allocatable dummy argument 'a=' without INTENT(IN)
+ real, allocatable :: a
+ !ERROR: NULL() actual argument 'NULL()' may not be associated with allocatable dummy argument dummy argument 'a=' that is INTENT(OUT) or INTENT(IN OUT)
+ call foo0(null())
+ !WARNING: NULL() actual argument 'NULL()' should not be associated with allocatable dummy argument dummy argument 'a=' without INTENT(IN)
call foo1(null())
!PORTABILITY: Allocatable dummy argument 'a=' is associated with NULL()
call foo2(null())
call foo3(null()) ! ok
+ !ERROR: Actual argument associated with INTENT(IN OUT) dummy argument 'a=' is not definable
+ !BECAUSE: 'null(mold=a)' is a null pointer
+ call foo0(null(mold=a))
+ !WARNING: A null pointer should not be associated with allocatable dummy argument 'a=' without INTENT(IN)
+ call foo1(null(mold=a))
+ !PORTABILITY: Allocatable dummy argument 'a=' is associated with a null pointer
+ call foo2(null(mold=a))
+ call foo3(null(mold=a)) ! ok
contains
+ subroutine foo0(a)
+ real, allocatable, intent(in out) :: a
+ end subroutine
subroutine foo1(a)
real, allocatable :: a
end subroutine
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have added some comments in the issue.
I think the warning message is still confusing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should lowering now generate conditional deallocation after the call in case the dummy argument associated to NULL is allocated inside the function?
Currently, lowering does not do that (relying on the INTENT(IN) aspect). But with this patch, the following code will now compile (with a warning) and leak memory:
module m
contains
subroutine foo(x)
real, allocatable :: x(:)
allocate(x(10))
end subroutine
end module
use m, only : foo
do i=1,10
call foo(null())
end do
end
valgrind:
==2914349== HEAP SUMMARY:
==2914349== in use at exit: 400 bytes in 10 blocks
==2914349== total heap usage: 10 allocs, 0 frees, 400 bytes allocated
==2914349==
==2914349== LEAK SUMMARY:
==2914349== definitely lost: 400 bytes in 10 blocks
I would think this is user's responsibility. |
Daniel and I are trying to figure that out. The question hinges on whether (If the result of that And this feature only works with XLF, and maybe we'll decide that it's not worth preserving. |
@DanielCChen ping -- should I merge this patch? |
I haven't got clear response from the standard committee as some of the folks are on vacation. |
No rush. I'll leave this one hanging for now. I prefer the status quo myself as well (allow only for INTENT(IN)). You can also leave the bug report open. |
…dummy We presently allow a NULL() actual argument to associate with a non-optional dummy allocatable argument only under INTENT(IN). This is too strict, as it precludes the case of a dummy argument with default intent. Continue to require that the actual argument be definable under INTENT(OUT) and INTENT(IN OUT), and (contra XLF) interpret NULL() as being an expression, not a definable variable, even when it is given an allocatable MOLD. Fixes llvm#115984. Fixes llvm#125928.
Ping; do you @DanielCChen have any objection to merging this fix? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
Thanks for the fix!
llvm#116126) …dummy We presently allow a NULL() actual argument to associate with a non-optional dummy allocatable argument only under INTENT(IN). This is too strict, as it precludes the case of a dummy argument with default intent. Continue to require that the actual argument be definable under INTENT(OUT) and INTENT(IN OUT), and (contra XLF) interpret NULL() as being an expression, not a definable variable, even when it is given an allocatable MOLD. Fixes llvm#115984.
…dummy
We presently allow a NULL() actual argument to associate with a non-optional dummy allocatable argument only under INTENT(IN). This is too strict, as it precludes the case of a dummy argument with default intent. Continue to require that the actual argument be definable under INTENT(OUT) and INTENT(IN OUT), and (contra XLF) interpret NULL() as being an expression, not a definable variable, even when it is given an allocatable MOLD.
Fixes #115984.