Fortran: different character lengths in array constructor [PR93289]
Checks
Commit Message
Dear all,
the attached patch downgrades different constant character lengths in an
array constructor from a GNU to a legacy extension, so that users get a
warning with -std=gnu. We continue to generate an error when standard
conformance is requested.
Regtested on x86_64-pc-linux-gnu (found one testcase where this
triggered... :)
OK for mainline?
Thanks,
Harald
Comments
On 2/1/25 10:25 AM, Harald Anlauf wrote:
> Dear all,
>
> the attached patch downgrades different constant character lengths in an
> array constructor from a GNU to a legacy extension, so that users get a
> warning with -std=gnu. We continue to generate an error when standard
> conformance is requested.
>
> Regtested on x86_64-pc-linux-gnu (found one testcase where this
> triggered... :)
>
> OK for mainline?
>
> Thanks,
> Harald
>
Yes, OK, and thanks
Jerry
On Sat, Feb 01, 2025 at 07:25:51PM +0100, Harald Anlauf wrote:
>
> the attached patch downgrades different constant character lengths in an
> array constructor from a GNU to a legacy extension, so that users get a
> warning with -std=gnu. We continue to generate an error when standard
> conformance is requested.
>
> Regtested on x86_64-pc-linux-gnu (found one testcase where this
> triggered... :)
>
> OK for mainline?
>
My vote is 'no'.
This is either a GNU extension or an error. It is certainly
not a legacy issue as array constructors simple cannot appear
old moldy *legacy* codes.
I would be in favor of making it a hard error. If you believe
gfortan must be able to compile invalid source, then add an option
such as -fallow-invalid-scalar-character-entities-in-array-constructor.
$0.02
Am 01.02.25 um 21:03 schrieb Steve Kargl:
> On Sat, Feb 01, 2025 at 07:25:51PM +0100, Harald Anlauf wrote:
>>
>> the attached patch downgrades different constant character lengths in an
>> array constructor from a GNU to a legacy extension, so that users get a
>> warning with -std=gnu. We continue to generate an error when standard
>> conformance is requested.
>>
>> Regtested on x86_64-pc-linux-gnu (found one testcase where this
>> triggered... :)
>>
>> OK for mainline?
>>
>
> My vote is 'no'.
>
> This is either a GNU extension or an error. It is certainly
> not a legacy issue as array constructors simple cannot appear
> old moldy *legacy* codes.
legacy /= moldy.
My intention is to downgrade existing, potentially dangerous
GNU extensions (like this one) carefully to "legacy", but not
with an axe.
> I would be in favor of making it a hard error. If you believe
> gfortan must be able to compile invalid source, then add an option
> such as -fallow-invalid-scalar-character-entities-in-array-constructor.
I don't see why we shall scare users by making code that is currently
accepted silently, because it is a GNU extension, suddenly to a hard
error.
So why must we be so tough?
> $0.02
>
Am 01.02.25 um 21:49 schrieb Harald Anlauf:
> So why must we be so tough?
Here's what I get from other compilers:
Intel, Nvidia, AMD flang: silent by default.
ifx -stand f18:
char_array_constructor_5.f90(11): warning #8208: If type specification
is omitted, each ac-value expression in the array constructor of type
CHARACTER must have the same length type parameters. ['ab']
print *, [str, "ab", "hjf333"] ! { dg-warning "must have the same length" }
---------------^
So no error, but Intel is known to often be on the loose side.
NAG: hard error.
That's what we get with -std=f*, too.
On Sat, Feb 01, 2025 at 09:49:17PM +0100, Harald Anlauf wrote:
> Am 01.02.25 um 21:03 schrieb Steve Kargl:
> > On Sat, Feb 01, 2025 at 07:25:51PM +0100, Harald Anlauf wrote:
> > >
> > > the attached patch downgrades different constant character lengths in an
> > > array constructor from a GNU to a legacy extension, so that users get a
> > > warning with -std=gnu. We continue to generate an error when standard
> > > conformance is requested.
> > >
> > > Regtested on x86_64-pc-linux-gnu (found one testcase where this
> > > triggered... :)
> > >
> > > OK for mainline?
> > >
> >
> > My vote is 'no'.
> >
> > This is either a GNU extension or an error. It is certainly
> > not a legacy issue as array constructors simple cannot appear
> > old moldy *legacy* codes.
>
> legacy /= moldy.
>
> My intention is to downgrade existing, potentially dangerous
> GNU extensions (like this one) carefully to "legacy", but not
> with an axe.
>
> > I would be in favor of making it a hard error. If you believe
> > gfortan must be able to compile invalid source, then add an option
> > such as -fallow-invalid-scalar-character-entities-in-array-constructor.
>
> I don't see why we shall scare users by making code that is currently
> accepted silently, because it is a GNU extension, suddenly to a hard
> error.
>
> So why must we be so tough?
>
Because -std=legacy allows a whole bunch of garbage.
Instead of fixing broken code, a user will slap -std=legacy
in a Makefile and move on. Then years from now, you'll see
-std=legacy in a whole bunch of Makefiles whether it is needed
or not. See -maligned-double and -fallow-argument-mismatch as
poster children.
Removing these types of extensions and legacy features
will hopefully make gfortran easier to maintain.
Again, just my $0.02.
Steve Kargl <sgk@troutmask.apl.washington.edu> writes:
> On Sat, Feb 01, 2025 at 09:49:17PM +0100, Harald Anlauf wrote:
>> Am 01.02.25 um 21:03 schrieb Steve Kargl:
>> > On Sat, Feb 01, 2025 at 07:25:51PM +0100, Harald Anlauf wrote:
>> > >
>> > > the attached patch downgrades different constant character lengths in an
>> > > array constructor from a GNU to a legacy extension, so that users get a
>> > > warning with -std=gnu. We continue to generate an error when standard
>> > > conformance is requested.
>> > >
>> > > Regtested on x86_64-pc-linux-gnu (found one testcase where this
>> > > triggered... :)
>> > >
>> > > OK for mainline?
>> > >
>> >
>> > My vote is 'no'.
>> >
>> > This is either a GNU extension or an error. It is certainly
>> > not a legacy issue as array constructors simple cannot appear
>> > old moldy *legacy* codes.
>>
>> legacy /= moldy.
>>
>> My intention is to downgrade existing, potentially dangerous
>> GNU extensions (like this one) carefully to "legacy", but not
>> with an axe.
>>
>> > I would be in favor of making it a hard error. If you believe
>> > gfortan must be able to compile invalid source, then add an option
>> > such as -fallow-invalid-scalar-character-entities-in-array-constructor.
>>
>> I don't see why we shall scare users by making code that is currently
>> accepted silently, because it is a GNU extension, suddenly to a hard
>> error.
>>
>> So why must we be so tough?
>>
>
> Because -std=legacy allows a whole bunch of garbage.
>
> Instead of fixing broken code, a user will slap -std=legacy
> in a Makefile and move on. Then years from now, you'll see
> -std=legacy in a whole bunch of Makefiles whether it is needed
> or not. See -maligned-double and -fallow-argument-mismatch as
> poster children.
I agree that this is what will happen. But for people running benchmarks,
it's kind-of (kind-of) a feature. Benchmarks tend to include relatively
old code by the time that they're released, and benchmarks continue to be
relevant (or at least widely tested) after they're out of maintenance.
So it has been really useful to have -std=legacy accept old, dangerous code,
since it means that we can continue to test old benchmarks with newer
compilers. Improving the benchmark source to avoid the dangerous constructs
would invalidate the test and make it harder to compare with historical
results.
> Again, just my $0.02.
Same here, just wanted to raise the benchmark use case.
Thanks,
Richard
On 2/3/25 2:49 AM, Richard Sandiford wrote:
> Steve Kargl <sgk@troutmask.apl.washington.edu> writes:
>> On Sat, Feb 01, 2025 at 09:49:17PM +0100, Harald Anlauf wrote:
>>> Am 01.02.25 um 21:03 schrieb Steve Kargl:
>>>> On Sat, Feb 01, 2025 at 07:25:51PM +0100, Harald Anlauf wrote:
>>>>>
>>>>> the attached patch downgrades different constant character lengths in an
>>>>> array constructor from a GNU to a legacy extension, so that users get a
>>>>> warning with -std=gnu. We continue to generate an error when standard
>>>>> conformance is requested.
>>>>>
>>>>> Regtested on x86_64-pc-linux-gnu (found one testcase where this
>>>>> triggered... :)
>>>>>
>>>>> OK for mainline?
>>>>>
>>>>
>>>> My vote is 'no'.
>>>>
>>>> This is either a GNU extension or an error. It is certainly
>>>> not a legacy issue as array constructors simple cannot appear
>>>> old moldy *legacy* codes.
>>>
>>> legacy /= moldy.
>>>
>>> My intention is to downgrade existing, potentially dangerous
>>> GNU extensions (like this one) carefully to "legacy", but not
>>> with an axe.
>>>
>>>> I would be in favor of making it a hard error. If you believe
>>>> gfortan must be able to compile invalid source, then add an option
>>>> such as -fallow-invalid-scalar-character-entities-in-array-constructor.
>>>
>>> I don't see why we shall scare users by making code that is currently
>>> accepted silently, because it is a GNU extension, suddenly to a hard
>>> error.
>>>
>>> So why must we be so tough?
>>>
>>
>> Because -std=legacy allows a whole bunch of garbage.
>>
>> Instead of fixing broken code, a user will slap -std=legacy
>> in a Makefile and move on. Then years from now, you'll see
>> -std=legacy in a whole bunch of Makefiles whether it is needed
>> or not. See -maligned-double and -fallow-argument-mismatch as
>> poster children.
>
> I agree that this is what will happen. But for people running benchmarks,
> it's kind-of (kind-of) a feature. Benchmarks tend to include relatively
> old code by the time that they're released, and benchmarks continue to be
> relevant (or at least widely tested) after they're out of maintenance.
>
> So it has been really useful to have -std=legacy accept old, dangerous code,
> since it means that we can continue to test old benchmarks with newer
> compilers. Improving the benchmark source to avoid the dangerous constructs
> would invalidate the test and make it harder to compare with historical
> results.
>
>> Again, just my $0.02.
>
> Same here, just wanted to raise the benchmark use case.
>
> Thanks,
> Richard
I think we have had good discussion and for sake of the good of the
order I recommend we push this for now. The work has been done.
Regards,
Jerry
Am 03.02.25 um 19:31 schrieb Jerry D:
> On 2/3/25 2:49 AM, Richard Sandiford wrote:
>> Steve Kargl <sgk@troutmask.apl.washington.edu> writes:
>>> On Sat, Feb 01, 2025 at 09:49:17PM +0100, Harald Anlauf wrote:
>>>> Am 01.02.25 um 21:03 schrieb Steve Kargl:
>>>>> On Sat, Feb 01, 2025 at 07:25:51PM +0100, Harald Anlauf wrote:
>>>>>>
>>>>>> the attached patch downgrades different constant character lengths
>>>>>> in an
>>>>>> array constructor from a GNU to a legacy extension, so that users
>>>>>> get a
>>>>>> warning with -std=gnu. We continue to generate an error when
>>>>>> standard
>>>>>> conformance is requested.
>>>>>>
>>>>>> Regtested on x86_64-pc-linux-gnu (found one testcase where this
>>>>>> triggered... :)
>>>>>>
>>>>>> OK for mainline?
>>>>>>
>>>>>
>>>>> My vote is 'no'.
>>>>>
>>>>> This is either a GNU extension or an error. It is certainly
>>>>> not a legacy issue as array constructors simple cannot appear
>>>>> old moldy *legacy* codes.
>>>>
>>>> legacy /= moldy.
>>>>
>>>> My intention is to downgrade existing, potentially dangerous
>>>> GNU extensions (like this one) carefully to "legacy", but not
>>>> with an axe.
>>>>
>>>>> I would be in favor of making it a hard error. If you believe
>>>>> gfortan must be able to compile invalid source, then add an option
>>>>> such as -fallow-invalid-scalar-character-entities-in-array-
>>>>> constructor.
>>>>
>>>> I don't see why we shall scare users by making code that is currently
>>>> accepted silently, because it is a GNU extension, suddenly to a hard
>>>> error.
>>>>
>>>> So why must we be so tough?
>>>>
>>>
>>> Because -std=legacy allows a whole bunch of garbage.
>>>
>>> Instead of fixing broken code, a user will slap -std=legacy
>>> in a Makefile and move on. Then years from now, you'll see
>>> -std=legacy in a whole bunch of Makefiles whether it is needed
>>> or not. See -maligned-double and -fallow-argument-mismatch as
>>> poster children.
>>
>> I agree that this is what will happen. But for people running
>> benchmarks,
>> it's kind-of (kind-of) a feature. Benchmarks tend to include relatively
>> old code by the time that they're released, and benchmarks continue to be
>> relevant (or at least widely tested) after they're out of maintenance.
>>
>> So it has been really useful to have -std=legacy accept old, dangerous
>> code,
>> since it means that we can continue to test old benchmarks with newer
>> compilers. Improving the benchmark source to avoid the dangerous
>> constructs
>> would invalidate the test and make it harder to compare with historical
>> results.
>>
>>> Again, just my $0.02.
>>
>> Same here, just wanted to raise the benchmark use case.
>>
>> Thanks,
>> Richard
>
> I think we have had good discussion and for sake of the good of the
> order I recommend we push this for now. The work has been done.
>
> Regards,
>
> Jerry
>
Thanks, Jerry!
Pushed: r15-7336-gf3a41e6cb5d70f
From c2ab5fc3e4cc758ca00e1878394c8f1466462f84 Mon Sep 17 00:00:00 2001
From: Harald Anlauf <anlauf@gmx.de>
Date: Sat, 1 Feb 2025 19:14:21 +0100
Subject: [PATCH] Fortran: different character lengths in array constructor
[PR93289]
PR fortran/93289
gcc/fortran/ChangeLog:
* decl.cc (gfc_set_constant_character_len): Downgrade different
string lengths in character array constructor to legacy extension.
gcc/testsuite/ChangeLog:
* gfortran.dg/unlimited_polymorphic_1.f03: Pad element in character
array constructor to correct length.
* gfortran.dg/char_array_constructor_5.f90: New test.
---
gcc/fortran/decl.cc | 20 +++++++++++++------
.../gfortran.dg/char_array_constructor_5.f90 | 13 ++++++++++++
.../gfortran.dg/unlimited_polymorphic_1.f03 | 2 +-
3 files changed, 28 insertions(+), 7 deletions(-)
create mode 100644 gcc/testsuite/gfortran.dg/char_array_constructor_5.f90
@@ -1889,12 +1889,20 @@ gfc_set_constant_character_len (gfc_charlen_t len, gfc_expr *expr,
/* Apply the standard by 'hand' otherwise it gets cleared for
initializers. */
- if (check_len != -1 && slen != check_len
- && !(gfc_option.allow_std & GFC_STD_GNU))
- gfc_error_now ("The CHARACTER elements of the array constructor "
- "at %L must have the same length (%ld/%ld)",
- &expr->where, (long) slen,
- (long) check_len);
+ if (check_len != -1 && slen != check_len)
+ {
+ if (!(gfc_option.allow_std & GFC_STD_GNU))
+ gfc_error_now ("The CHARACTER elements of the array constructor "
+ "at %L must have the same length (%ld/%ld)",
+ &expr->where, (long) slen,
+ (long) check_len);
+ else
+ gfc_notify_std (GFC_STD_LEGACY,
+ "The CHARACTER elements of the array constructor "
+ "at %L must have the same length (%ld/%ld)",
+ &expr->where, (long) slen,
+ (long) check_len);
+ }
s[len] = '\0';
free (expr->value.character.string);
new file mode 100644
@@ -0,0 +1,13 @@
+! { dg-do compile }
+! { dg-additional-options "-std=gnu" }
+!
+! PR fortran/93289
+!
+! Contributed by Tobias Burnus
+
+character(len=*), parameter :: str = "abj", str2 = "1234"
+print *, [character(5) :: str, "ab", "hjf333"]
+print *, [character(5) :: str, str2]
+print *, [str, "ab", "hjf333"] ! { dg-warning "must have the same length" }
+print *, [str, str2] ! { dg-warning "must have the same length" }
+end
@@ -155,7 +155,7 @@ END MODULE
call foo([a(8),a(9)], res)
if (trim (res) .ne. "type(a) array 8 9") STOP 1
- call foo([sun, " & rain"], res)
+ call foo([sun, " & rain "], res)
if (trim (res) .ne. "char( 8, 2)sunshine & rain") STOP 1
call foo([sun//" never happens", " & rain always happens"], res)
--
2.43.0