Skip to content

[flang] Adjust semantics of the char length of an array constructor #97337

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

Merged
merged 1 commit into from
Jul 11, 2024

Conversation

klausler
Copy link
Contributor

@klausler klausler commented Jul 1, 2024

An implied DO loop with no trips in an array constructor does not have a well-defined character length unless its data items have a length that is constant expression. That works, but the implementation is too broadly applied. An array constructor with an explicit type-spec always has a well-defined length.

An implied DO loop with no trips in an array constructor does not have
a well-defined character length unless its data items have a length that
is constant expression.  That works, but the implementation is too broadly
applied.  An array constructor with an explicit type-spec always has a
well-defined length.
@klausler klausler requested a review from vzakhari July 1, 2024 19:11
@llvmbot llvmbot added flang Flang issues not falling into any other category flang:fir-hlfir flang:semantics labels Jul 1, 2024
@llvmbot
Copy link
Member

llvmbot commented Jul 1, 2024

@llvm/pr-subscribers-flang-fir-hlfir

@llvm/pr-subscribers-flang-semantics

Author: Peter Klausler (klausler)

Changes

An implied DO loop with no trips in an array constructor does not have a well-defined character length unless its data items have a length that is constant expression. That works, but the implementation is too broadly applied. An array constructor with an explicit type-spec always has a well-defined length.


Full diff: https://github.com/llvm/llvm-project/pull/97337.diff

3 Files Affected:

  • (modified) flang/lib/Semantics/expression.cpp (+2-1)
  • (modified) flang/test/Lower/HLFIR/array-ctor-character.f90 (+5-4)
  • (modified) flang/test/Semantics/array-constr-len.f90 (-1)
diff --git a/flang/lib/Semantics/expression.cpp b/flang/lib/Semantics/expression.cpp
index 2202639a92e43..897e94966a134 100644
--- a/flang/lib/Semantics/expression.cpp
+++ b/flang/lib/Semantics/expression.cpp
@@ -1584,7 +1584,8 @@ class ArrayConstructorContext {
   std::optional<Expr<SubscriptInteger>> LengthIfGood() const {
     if (type_) {
       auto len{type_->LEN()};
-      if (len && IsConstantExpr(*len) && !ContainsAnyImpliedDoIndex(*len)) {
+      if (explicitType_ ||
+          (len && IsConstantExpr(*len) && !ContainsAnyImpliedDoIndex(*len))) {
         return len;
       }
     }
diff --git a/flang/test/Lower/HLFIR/array-ctor-character.f90 b/flang/test/Lower/HLFIR/array-ctor-character.f90
index 85e6ed27fe077..881085b370ffe 100644
--- a/flang/test/Lower/HLFIR/array-ctor-character.f90
+++ b/flang/test/Lower/HLFIR/array-ctor-character.f90
@@ -93,10 +93,11 @@ subroutine test_set_length_sanitize(i, c1)
   call takes_char([character(len=i):: c1])
 end subroutine
 ! CHECK-LABEL:   func.func @_QPtest_set_length_sanitize(
-! CHECK:   %[[VAL_6:.*]]:2 = hlfir.declare {{.*}}Ec1
-! CHECK:   %[[VAL_9:.*]]:2 = hlfir.declare {{.*}}Ei
-! CHECK:   %[[VAL_25:.*]] = fir.load %[[VAL_9]]#0 : !fir.ref<i64>
+! CHECK:   %[[VAL_2:.*]]:2 = hlfir.declare {{.*}}Ec1
+! CHECK:   %[[VAL_3:.*]]:2 = hlfir.declare %arg0
+! CHECK:   %[[VAL_4:.*]] = fir.load %[[VAL_3]]#0 : !fir.ref<i64>
+! CHECK:   %[[VAL_25:.*]] = fir.load %[[VAL_3]]#0 : !fir.ref<i64>
 ! CHECK:   %[[VAL_26:.*]] = arith.constant 0 : i64
 ! CHECK:   %[[VAL_27:.*]] = arith.cmpi sgt, %[[VAL_25]], %[[VAL_26]] : i64
 ! CHECK:   %[[VAL_28:.*]] = arith.select %[[VAL_27]], %[[VAL_25]], %[[VAL_26]] : i64
-! CHECK:   %[[VAL_29:.*]] = hlfir.set_length %[[VAL_6]]#0 len %[[VAL_28]] : (!fir.boxchar<1>, i64) -> !hlfir.expr<!fir.char<1,?>>
+! CHECK:   %[[VAL_29:.*]] = hlfir.set_length %[[VAL_2]]#0 len %[[VAL_28]] : (!fir.boxchar<1>, i64) -> !hlfir.expr<!fir.char<1,?>>
diff --git a/flang/test/Semantics/array-constr-len.f90 b/flang/test/Semantics/array-constr-len.f90
index a785c9e2ece6d..4de9c76c7041c 100644
--- a/flang/test/Semantics/array-constr-len.f90
+++ b/flang/test/Semantics/array-constr-len.f90
@@ -10,6 +10,5 @@ subroutine subr(s,n)
   print *, [(s(1:j),j=1,0)]
   print *, [(s(1:1),j=1,0)] ! ok
   print *, [character(2)::(s(1:n),j=1,0)] ! ok
-  !ERROR: Array constructor implied DO loop has no iterations and indeterminate character length
   print *, [character(n)::(s(1:n),j=1,0)]
 end

@klausler klausler merged commit 2d7e136 into llvm:main Jul 11, 2024
11 checks passed
@klausler klausler deleted the bug1286 branch July 11, 2024 19:30
DavidSpickett added a commit that referenced this pull request Jul 12, 2024
…ructor" (#98612)

Reverts #97337

This has caused llvm test suite failures on our bots, for example:
https://lab.llvm.org/buildbot/#/builders/17/builds/709

```
FAIL: test-suite::gfortran-regression-execute-regression__char_length_21_f90.test 
FAIL: test-suite::gfortran-regression-execute-regression__char_length_20_f90.test
```
@DavidSpickett
Copy link
Collaborator

I have reverted this due to test suite failures: https://lab.llvm.org/buildbot/#/builders/17/builds/709

Also it would help if you set a email address for your commits so that you are able to receive buildbot reports. Buildbots do now report on PRs directly, but only if they build a single commit which is rare for our test suite bots.

klausler added a commit to klausler/llvm-project that referenced this pull request Jul 12, 2024
Pull request llvm#97337 was
reverted by llvm#98612 due
to two failing tests in llvm-test-suite -- which I ran, as always,
but must have bungled or misinterpreted (mea culpa).

The failing tests were llvm-test-suite/Fortran/gfortran/regression/
char_length_{20,21}.f90.  They have array constructors with
explicit character types whose dynamic length values are negative
at runtime, which must be interpreted as zero.

This patch extends the original to cover those cases.
klausler added a commit to klausler/llvm-project that referenced this pull request Jul 12, 2024
Pull request llvm#97337 was
reverted by llvm#98612 due
to two failing tests in llvm-test-suite -- which I ran, as always,
but must have bungled or misinterpreted (mea culpa).

The failing tests were llvm-test-suite/Fortran/gfortran/regression/
char_length_{20,21}.f90.  They have array constructors with
explicit character types whose dynamic length values are negative
at runtime and which must be interpreted as zero.

This patch extends the original to cover those cases.
klausler added a commit that referenced this pull request Jul 12, 2024
Pull request #97337 was
reverted by #98612 due
to two failing tests in llvm-test-suite -- which I ran, as always,
but must have bungled or misinterpreted (mea culpa).
    
The failing tests were llvm-test-suite/Fortran/gfortran/regression/
char_length_{20,21}.f90.  They have array constructors with
explicit character types whose dynamic length values are negative
at runtime, which must be interpreted as zero.
    
This patch extends the original to cover those cases.
aaryanshukla pushed a commit to aaryanshukla/llvm-project that referenced this pull request Jul 14, 2024
…lvm#97337)

An implied DO loop with no trips in an array constructor does not have a
well-defined character length unless its data items have a length that
is constant expression. That works, but the implementation is too
broadly applied. An array constructor with an explicit type-spec always
has a well-defined length.
aaryanshukla pushed a commit to aaryanshukla/llvm-project that referenced this pull request Jul 14, 2024
…ructor" (llvm#98612)

Reverts llvm#97337

This has caused llvm test suite failures on our bots, for example:
https://lab.llvm.org/buildbot/#/builders/17/builds/709

```
FAIL: test-suite::gfortran-regression-execute-regression__char_length_21_f90.test 
FAIL: test-suite::gfortran-regression-execute-regression__char_length_20_f90.test
```
aaryanshukla pushed a commit to aaryanshukla/llvm-project that referenced this pull request Jul 14, 2024
Pull request llvm#97337 was
reverted by llvm#98612 due
to two failing tests in llvm-test-suite -- which I ran, as always,
but must have bungled or misinterpreted (mea culpa).
    
The failing tests were llvm-test-suite/Fortran/gfortran/regression/
char_length_{20,21}.f90.  They have array constructors with
explicit character types whose dynamic length values are negative
at runtime, which must be interpreted as zero.
    
This patch extends the original to cover those cases.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flang:fir-hlfir flang:semantics flang Flang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants