-
Notifications
You must be signed in to change notification settings - Fork 15.1k
[flang] Fixed regression with CDEFINED linkage #164616
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#162722 introduced a regression that started creating initializers for CDEFINED variables. CDEFINED variables cannot have initializers, because their storage is expected come from elsewhere, likely outside of Fortran. Fixed the regression and improved the regression test to catch the incorrect initialization case.
|
@llvm/pr-subscribers-flang-fir-hlfir @llvm/pr-subscribers-flang-semantics Author: Eugene Epshteyn (eugeneepshteyn) Changes#162722 introduced a regression that started creating initializers for CDEFINED variables. CDEFINED variables cannot have initializers, because their storage is expected come from elsewhere, likely outside of Fortran. Fixed the regression and improved the regression test to catch the incorrect initialization case. Full diff: https://github.com/llvm/llvm-project/pull/164616.diff 3 Files Affected:
diff --git a/flang/lib/Semantics/resolve-names.cpp b/flang/lib/Semantics/resolve-names.cpp
index db75437708a6c..5cb1bc636a582 100644
--- a/flang/lib/Semantics/resolve-names.cpp
+++ b/flang/lib/Semantics/resolve-names.cpp
@@ -9194,11 +9194,12 @@ bool DeclarationVisitor::CheckNonPointerInitialization(
"'%s' has already been initialized"_err_en_US);
} else if (IsAllocatable(ultimate)) {
Say(name, "Allocatable object '%s' cannot be initialized"_err_en_US);
+ } else if (details->isCDefined()) {
+ // CDEFINED variables cannot have initializer, because their storage
+ // may come outside of Fortran.
+ context().Warn(common::UsageWarning::CdefinedInit, name.source,
+ "CDEFINED variable cannot be initialized"_warn_en_US);
} else {
- if (details->isCDefined()) {
- context().Warn(common::UsageWarning::CdefinedInit, name.source,
- "CDEFINED variable should not have an initializer"_warn_en_US);
- }
return true;
}
} else {
diff --git a/flang/test/Lower/cdefined.f90 b/flang/test/Lower/cdefined.f90
index 89599442589eb..87a01b51460fc 100644
--- a/flang/test/Lower/cdefined.f90
+++ b/flang/test/Lower/cdefined.f90
@@ -6,4 +6,5 @@ module m
integer(c_int), bind(C, name='c_global', CDEFINED) :: c = 42
! CHECK: fir.global @c_global : i32
! CHECK-NOT: fir.zero_bits
+ ! CHECK-NOT: arith.constant 42
end
diff --git a/flang/test/Semantics/cdefined.f90 b/flang/test/Semantics/cdefined.f90
index 84103ce661d0b..9e0b0a485ee15 100644
--- a/flang/test/Semantics/cdefined.f90
+++ b/flang/test/Semantics/cdefined.f90
@@ -1,6 +1,6 @@
! RUN: %python %S/test_errors.py %s %flang_fc1 -pedantic -Werror
module m
use iso_c_binding
- !WARNING: CDEFINED variable should not have an initializer [-Wcdefined-init]
+ !WARNING: CDEFINED variable cannot be initialized [-Wcdefined-init]
integer(c_int), bind(C, name='c_global', CDEFINED) :: c = 42
end
|
| } else if (details->isCDefined()) { | ||
| // CDEFINED variables cannot have initializer, because their storage | ||
| // may come outside of Fortran. | ||
| context().Warn(common::UsageWarning::CdefinedInit, name.source, |
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.
If they can't be initialized, this should be a hard error.
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.
Done.
llvm#162722 introduced a regression that started creating initializers for CDEFINED variables. CDEFINED variables cannot have initializers, because their storage is expected come from elsewhere, likely outside of Fortran. Fixed the regression and improved the regression test to catch the incorrect initialization case. Also, based on the code review feedback, made CDEFINED variable initialization a hard error and updated tests accordingly.
llvm#162722 introduced a regression that started creating initializers for CDEFINED variables. CDEFINED variables cannot have initializers, because their storage is expected come from elsewhere, likely outside of Fortran. Fixed the regression and improved the regression test to catch the incorrect initialization case. Also, based on the code review feedback, made CDEFINED variable initialization a hard error and updated tests accordingly.
#162722 introduced a regression that started creating initializers for CDEFINED variables. CDEFINED variables cannot have initializers, because their storage is expected come from elsewhere, likely outside of Fortran. Fixed the regression and improved the regression test to catch the incorrect initialization case.
Also, based on the code review feedback, made CDEFINED variable initialization a hard error and updated tests accordingly.