Fortran: improve attribute conflict checking [PR93635]

Message ID trinity-4fff01df-2b22-4df8-883f-6f60e0b15a41-1715023981023@3c-app-gmx-bs33
State New
Headers
Series Fortran: improve attribute conflict checking [PR93635] |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gcc_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gcc_check--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gcc_check--master-aarch64 success Testing passed

Commit Message

Harald Anlauf May 6, 2024, 7:33 p.m. UTC
  Dear all,

I've been contemplating whether to submit the attached patch.
It addresses an ICE-on-invalid as reported in the PR, and also
fixes an accepts-invalid (see testcase), plus maybe some more,
related due to incomplete checking of symbol attribute conflicts.

The fix does not fully address the general issue, which is
analyzed by Steve: some of the checks do depend on the selected
Fortran standard, and under circumstances such as in the testcase
the checking of other, standard-version-independent conflicts
simply does not occur.

Steve's solution would fix that, but unfortunately leads to issues
with error recovery in notoriously fragile parts of the FE: e.g.
testcase pr87907.f90 needs adjusting, and minor variations
of it will lead to various other horrendous ICEs that remind
of existing PRs where parsing or resolution goes sideways.

I therefore propose a much simpler approach: move - if possible -
selected of the standard-version-dependent checks after the
version-independent ones.  I think this could help in getting more
consistent error reporting and recovery.  However, I did *not*
move those checks that are critical when processing interfaces.
(-> pr87907.f90 / (sub)modules)

The patch therefore does not require any testsuite update and
should not give any other surprises, so it should be very safe.
The plan is also to leave the PR open for the time being.

Regtesting on x86_64-pc-linux-gnu.  OK for mainline?

Thanks,
Harald
  

Comments

Mikael Morin May 9, 2024, 7:51 p.m. UTC | #1
Hello,

Le 06/05/2024 à 21:33, Harald Anlauf a écrit :
> Dear all,
> 
> I've been contemplating whether to submit the attached patch.
> It addresses an ICE-on-invalid as reported in the PR, and also
> fixes an accepts-invalid (see testcase), plus maybe some more,
> related due to incomplete checking of symbol attribute conflicts.
> 
> The fix does not fully address the general issue, which is
> analyzed by Steve: some of the checks do depend on the selected
> Fortran standard, and under circumstances such as in the testcase
> the checking of other, standard-version-independent conflicts
> simply does not occur.
> 
> Steve's solution would fix that, but unfortunately leads to issues
> with error recovery in notoriously fragile parts of the FE: e.g.
> testcase pr87907.f90 needs adjusting, and minor variations
> of it will lead to various other horrendous ICEs that remind
> of existing PRs where parsing or resolution goes sideways.
> 
> I therefore propose a much simpler approach: move - if possible -
> selected of the standard-version-dependent checks after the
> version-independent ones.  I think this could help in getting more
> consistent error reporting and recovery.  However, I did *not*
> move those checks that are critical when processing interfaces.
> (-> pr87907.f90 / (sub)modules)
> 
Your patch looks clean, but I'm concerned that the order of the checks 
should be the important ones first, regardless of their standard 
version.  I'm trying to look at the ICE caused by your other tentative 
patch at https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93635#c6 but I 
can't reproduce the problem.  Do you by any chance have around some of 
the variations causing "horrendous" ICEs?

> The patch therefore does not require any testsuite update and
> should not give any other surprises, so it should be very safe.
> The plan is also to leave the PR open for the time being.
> 
> Regtesting on x86_64-pc-linux-gnu.  OK for mainline?
> 
> Thanks,
> Harald
>
  
Harald Anlauf May 9, 2024, 8:30 p.m. UTC | #2
Hi Mikael,

Am 09.05.24 um 21:51 schrieb Mikael Morin:
> Hello,
>
> Le 06/05/2024 à 21:33, Harald Anlauf a écrit :
>> Dear all,
>>
>> I've been contemplating whether to submit the attached patch.
>> It addresses an ICE-on-invalid as reported in the PR, and also
>> fixes an accepts-invalid (see testcase), plus maybe some more,
>> related due to incomplete checking of symbol attribute conflicts.
>>
>> The fix does not fully address the general issue, which is
>> analyzed by Steve: some of the checks do depend on the selected
>> Fortran standard, and under circumstances such as in the testcase
>> the checking of other, standard-version-independent conflicts
>> simply does not occur.
>>
>> Steve's solution would fix that, but unfortunately leads to issues
>> with error recovery in notoriously fragile parts of the FE: e.g.
>> testcase pr87907.f90 needs adjusting, and minor variations
>> of it will lead to various other horrendous ICEs that remind
>> of existing PRs where parsing or resolution goes sideways.
>>
>> I therefore propose a much simpler approach: move - if possible -
>> selected of the standard-version-dependent checks after the
>> version-independent ones.  I think this could help in getting more
>> consistent error reporting and recovery.  However, I did *not*
>> move those checks that are critical when processing interfaces.
>> (-> pr87907.f90 / (sub)modules)
>>
> Your patch looks clean, but I'm concerned that the order of the checks
> should be the important ones first, regardless of their standard
> version.  I'm trying to look at the ICE caused by your other tentative
> patch at https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93635#c6 but I
> can't reproduce the problem.  Do you by any chance have around some of
> the variations causing "horrendous" ICEs?

Oh, that's easy.  Just move the block

   conf_std (allocatable, dummy, GFC_STD_F2003);
   conf_std (allocatable, function, GFC_STD_F2003);
   conf_std (allocatable, result, GFC_STD_F2003);

towards the end of the gfc_check_conflict before the return true.

While the error messages for the original gfortran.dg/pr87907.f90
look harmless, commenting out the main program p I get:

pr87907.f90:15:18:

    15 |       subroutine g(x)   ! { dg-error "mismatch in argument" }
       |                  1
Error: FUNCTION attribute conflicts with SUBROUTINE attribute in 'g' at (1)
f951: internal compiler error: Segmentation fault
0x13b8ec2 crash_signal
         ../../gcc-trunk/gcc/toplev.cc:319
0xba530e free_sym_tree
         ../../gcc-trunk/gcc/fortran/symbol.cc:4026
0xba5319 free_sym_tree
         ../../gcc-trunk/gcc/fortran/symbol.cc:4026
0xba5319 free_sym_tree
         ../../gcc-trunk/gcc/fortran/symbol.cc:4026
0xba5319 free_sym_tree
         ../../gcc-trunk/gcc/fortran/symbol.cc:4026
0xba5609 gfc_free_namespace(gfc_namespace*&)
         ../../gcc-trunk/gcc/fortran/symbol.cc:4168
0xba39c1 gfc_free_symbol(gfc_symbol*&)
         ../../gcc-trunk/gcc/fortran/symbol.cc:3173
0xba3b89 gfc_release_symbol(gfc_symbol*&)
         ../../gcc-trunk/gcc/fortran/symbol.cc:3216
0xba5339 free_sym_tree
         ../../gcc-trunk/gcc/fortran/symbol.cc:4029
0xba5609 gfc_free_namespace(gfc_namespace*&)
         ../../gcc-trunk/gcc/fortran/symbol.cc:4168
0xba58ef gfc_symbol_done_2()
         ../../gcc-trunk/gcc/fortran/symbol.cc:4236
0xb12ec8 gfc_done_2()
         ../../gcc-trunk/gcc/fortran/misc.cc:387
0xb4ac7f clean_up_modules
         ../../gcc-trunk/gcc/fortran/parse.cc:7057
0xb4af02 translate_all_program_units
         ../../gcc-trunk/gcc/fortran/parse.cc:7122
0xb4b735 gfc_parse_file()
         ../../gcc-trunk/gcc/fortran/parse.cc:7413
0xbb626f gfc_be_parse_file
         ../../gcc-trunk/gcc/fortran/f95-lang.cc:241


Restoring the main program but simply adding "end subroutine g"
where it is naively expected gives:

pr87907.f90:15:18:

    15 |       subroutine g(x)   ! { dg-error "mismatch in argument" }
       |                  1
Error: FUNCTION attribute conflicts with SUBROUTINE attribute in 'g' at (1)
pr87907.f90:16:9:

    16 |       end subroutine g
       |         1
Error: Expecting END SUBMODULE statement at (1)
pr87907.f90:20:7:

    20 |    use m                ! { dg-error "has a type" }
       |       1
    21 |    integer :: x = 3
    22 |    call g(x)            ! { dg-error "which is not consistent
with" }
       |
     2
Error: 'g' at (1) has a type, which is not consistent with the CALL at (2)
f951: internal compiler error: in gfc_free_namespace, at
fortran/symbol.cc:4164
0xba55e1 gfc_free_namespace(gfc_namespace*&)
         ../../gcc-trunk/gcc/fortran/symbol.cc:4164
0xba39c1 gfc_free_symbol(gfc_symbol*&)
         ../../gcc-trunk/gcc/fortran/symbol.cc:3173
0xba3b89 gfc_release_symbol(gfc_symbol*&)
         ../../gcc-trunk/gcc/fortran/symbol.cc:3216
0xba5339 free_sym_tree
         ../../gcc-trunk/gcc/fortran/symbol.cc:4029
0xba5609 gfc_free_namespace(gfc_namespace*&)
         ../../gcc-trunk/gcc/fortran/symbol.cc:4168
0xba58ef gfc_symbol_done_2()
         ../../gcc-trunk/gcc/fortran/symbol.cc:4236
0xb12ec8 gfc_done_2()
         ../../gcc-trunk/gcc/fortran/misc.cc:387
0xb4ac7f clean_up_modules
         ../../gcc-trunk/gcc/fortran/parse.cc:7057
0xb4af02 translate_all_program_units
         ../../gcc-trunk/gcc/fortran/parse.cc:7122
0xb4b735 gfc_parse_file()
         ../../gcc-trunk/gcc/fortran/parse.cc:7413
0xbb626f gfc_be_parse_file
         ../../gcc-trunk/gcc/fortran/f95-lang.cc:241

Now adding -std=f2018 to the compiler flags I get:

pr87907.f90:15:18:

    15 |       subroutine g(x)   ! { dg-error "mismatch in argument" }
       |                  1
Error: FUNCTION attribute conflicts with SUBROUTINE attribute in 'g' at (1)
pr87907.f90:16:9:

    16 |       end subroutine g
       |         1
Error: Expecting END SUBMODULE statement at (1)
pr87907.f90:20:7:

    20 |    use m                ! { dg-error "has a type" }
       |       1
    21 |    integer :: x = 3
    22 |    call g(x)            ! { dg-error "which is not consistent
with" }
       |
     2
Error: 'g' at (1) has a type, which is not consistent with the CALL at (2)
free(): invalid pointer
f951: internal compiler error: Aborted
0x13b8ec2 crash_signal
         ../../gcc-trunk/gcc/toplev.cc:319
0xba584f gfc_free_namespace(gfc_namespace*&)
         ../../gcc-trunk/gcc/fortran/symbol.cc:4207
0xba39c1 gfc_free_symbol(gfc_symbol*&)
         ../../gcc-trunk/gcc/fortran/symbol.cc:3173
0xba3b89 gfc_release_symbol(gfc_symbol*&)
         ../../gcc-trunk/gcc/fortran/symbol.cc:3216
0xba5339 free_sym_tree
         ../../gcc-trunk/gcc/fortran/symbol.cc:4029
0xba5609 gfc_free_namespace(gfc_namespace*&)
         ../../gcc-trunk/gcc/fortran/symbol.cc:4168
0xba58ef gfc_symbol_done_2()
         ../../gcc-trunk/gcc/fortran/symbol.cc:4236
0xb12ec8 gfc_done_2()
         ../../gcc-trunk/gcc/fortran/misc.cc:387
0xb4ac7f clean_up_modules
         ../../gcc-trunk/gcc/fortran/parse.cc:7057
0xb4af02 translate_all_program_units
         ../../gcc-trunk/gcc/fortran/parse.cc:7122
0xb4b735 gfc_parse_file()
         ../../gcc-trunk/gcc/fortran/parse.cc:7413
0xbb626f gfc_be_parse_file
         ../../gcc-trunk/gcc/fortran/f95-lang.cc:241

I'll stop here...

We currently do not recover well from errors, and the prevention
of corrupted namespaces is apparently a goal we aim at.

Cheers,
Harald

>> The patch therefore does not require any testsuite update and
>> should not give any other surprises, so it should be very safe.
>> The plan is also to leave the PR open for the time being.
>>
>> Regtesting on x86_64-pc-linux-gnu.  OK for mainline?
>>
>> Thanks,
>> Harald
>>
>
>
  
Mikael Morin May 10, 2024, 9:45 a.m. UTC | #3
Le 09/05/2024 à 22:30, Harald Anlauf a écrit :
> Hi Mikael,
> 
> Am 09.05.24 um 21:51 schrieb Mikael Morin:
>> Hello,
>>
>> Le 06/05/2024 à 21:33, Harald Anlauf a écrit :
>>> Dear all,
>>>
>>> I've been contemplating whether to submit the attached patch.
>>> It addresses an ICE-on-invalid as reported in the PR, and also
>>> fixes an accepts-invalid (see testcase), plus maybe some more,
>>> related due to incomplete checking of symbol attribute conflicts.
>>>
>>> The fix does not fully address the general issue, which is
>>> analyzed by Steve: some of the checks do depend on the selected
>>> Fortran standard, and under circumstances such as in the testcase
>>> the checking of other, standard-version-independent conflicts
>>> simply does not occur.
>>>
>>> Steve's solution would fix that, but unfortunately leads to issues
>>> with error recovery in notoriously fragile parts of the FE: e.g.
>>> testcase pr87907.f90 needs adjusting, and minor variations
>>> of it will lead to various other horrendous ICEs that remind
>>> of existing PRs where parsing or resolution goes sideways.
>>>
>>> I therefore propose a much simpler approach: move - if possible -
>>> selected of the standard-version-dependent checks after the
>>> version-independent ones.  I think this could help in getting more
>>> consistent error reporting and recovery.  However, I did *not*
>>> move those checks that are critical when processing interfaces.
>>> (-> pr87907.f90 / (sub)modules)
>>>
>> Your patch looks clean, but I'm concerned that the order of the checks
>> should be the important ones first, regardless of their standard
>> version.  I'm trying to look at the ICE caused by your other tentative
>> patch at https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93635#c6 but I
>> can't reproduce the problem.  Do you by any chance have around some of
>> the variations causing "horrendous" ICEs?
> 
> Oh, that's easy.  Just move the block
> 
>    conf_std (allocatable, dummy, GFC_STD_F2003);
>    conf_std (allocatable, function, GFC_STD_F2003);
>    conf_std (allocatable, result, GFC_STD_F2003);
> 
> towards the end of the gfc_check_conflict before the return true.
> 
> While the error messages for the original gfortran.dg/pr87907.f90
> look harmless, commenting out the main program p I get:
> 
> pr87907.f90:15:18:
> 
>     15 |       subroutine g(x)   ! { dg-error "mismatch in argument" }
>        |                  1
> Error: FUNCTION attribute conflicts with SUBROUTINE attribute in 'g' at (1)
> f951: internal compiler error: Segmentation fault
> 0x13b8ec2 crash_signal
>          ../../gcc-trunk/gcc/toplev.cc:319
> 0xba530e free_sym_tree
>          ../../gcc-trunk/gcc/fortran/symbol.cc:4026
> 0xba5319 free_sym_tree
>          ../../gcc-trunk/gcc/fortran/symbol.cc:4026
> 0xba5319 free_sym_tree
>          ../../gcc-trunk/gcc/fortran/symbol.cc:4026
> 0xba5319 free_sym_tree
>          ../../gcc-trunk/gcc/fortran/symbol.cc:4026
> 0xba5609 gfc_free_namespace(gfc_namespace*&)
>          ../../gcc-trunk/gcc/fortran/symbol.cc:4168
> 0xba39c1 gfc_free_symbol(gfc_symbol*&)
>          ../../gcc-trunk/gcc/fortran/symbol.cc:3173
> 0xba3b89 gfc_release_symbol(gfc_symbol*&)
>          ../../gcc-trunk/gcc/fortran/symbol.cc:3216
> 0xba5339 free_sym_tree
>          ../../gcc-trunk/gcc/fortran/symbol.cc:4029
> 0xba5609 gfc_free_namespace(gfc_namespace*&)
>          ../../gcc-trunk/gcc/fortran/symbol.cc:4168
> 0xba58ef gfc_symbol_done_2()
>          ../../gcc-trunk/gcc/fortran/symbol.cc:4236
> 0xb12ec8 gfc_done_2()
>          ../../gcc-trunk/gcc/fortran/misc.cc:387
> 0xb4ac7f clean_up_modules
>          ../../gcc-trunk/gcc/fortran/parse.cc:7057
> 0xb4af02 translate_all_program_units
>          ../../gcc-trunk/gcc/fortran/parse.cc:7122
> 0xb4b735 gfc_parse_file()
>          ../../gcc-trunk/gcc/fortran/parse.cc:7413
> 0xbb626f gfc_be_parse_file
>          ../../gcc-trunk/gcc/fortran/f95-lang.cc:241
> 
> 
> Restoring the main program but simply adding "end subroutine g"
> where it is naively expected gives:
> 
> pr87907.f90:15:18:
> 
>     15 |       subroutine g(x)   ! { dg-error "mismatch in argument" }
>        |                  1
> Error: FUNCTION attribute conflicts with SUBROUTINE attribute in 'g' at (1)
> pr87907.f90:16:9:
> 
>     16 |       end subroutine g
>        |         1
> Error: Expecting END SUBMODULE statement at (1)
> pr87907.f90:20:7:
> 
>     20 |    use m                ! { dg-error "has a type" }
>        |       1
>     21 |    integer :: x = 3
>     22 |    call g(x)            ! { dg-error "which is not consistent
> with" }
>        |
>      2
> Error: 'g' at (1) has a type, which is not consistent with the CALL at (2)
> f951: internal compiler error: in gfc_free_namespace, at
> fortran/symbol.cc:4164
> 0xba55e1 gfc_free_namespace(gfc_namespace*&)
>          ../../gcc-trunk/gcc/fortran/symbol.cc:4164
> 0xba39c1 gfc_free_symbol(gfc_symbol*&)
>          ../../gcc-trunk/gcc/fortran/symbol.cc:3173
> 0xba3b89 gfc_release_symbol(gfc_symbol*&)
>          ../../gcc-trunk/gcc/fortran/symbol.cc:3216
> 0xba5339 free_sym_tree
>          ../../gcc-trunk/gcc/fortran/symbol.cc:4029
> 0xba5609 gfc_free_namespace(gfc_namespace*&)
>          ../../gcc-trunk/gcc/fortran/symbol.cc:4168
> 0xba58ef gfc_symbol_done_2()
>          ../../gcc-trunk/gcc/fortran/symbol.cc:4236
> 0xb12ec8 gfc_done_2()
>          ../../gcc-trunk/gcc/fortran/misc.cc:387
> 0xb4ac7f clean_up_modules
>          ../../gcc-trunk/gcc/fortran/parse.cc:7057
> 0xb4af02 translate_all_program_units
>          ../../gcc-trunk/gcc/fortran/parse.cc:7122
> 0xb4b735 gfc_parse_file()
>          ../../gcc-trunk/gcc/fortran/parse.cc:7413
> 0xbb626f gfc_be_parse_file
>          ../../gcc-trunk/gcc/fortran/f95-lang.cc:241
> 
> Now adding -std=f2018 to the compiler flags I get:
> 
> pr87907.f90:15:18:
> 
>     15 |       subroutine g(x)   ! { dg-error "mismatch in argument" }
>        |                  1
> Error: FUNCTION attribute conflicts with SUBROUTINE attribute in 'g' at (1)
> pr87907.f90:16:9:
> 
>     16 |       end subroutine g
>        |         1
> Error: Expecting END SUBMODULE statement at (1)
> pr87907.f90:20:7:
> 
>     20 |    use m                ! { dg-error "has a type" }
>        |       1
>     21 |    integer :: x = 3
>     22 |    call g(x)            ! { dg-error "which is not consistent
> with" }
>        |
>      2
> Error: 'g' at (1) has a type, which is not consistent with the CALL at (2)
> free(): invalid pointer
> f951: internal compiler error: Aborted
> 0x13b8ec2 crash_signal
>          ../../gcc-trunk/gcc/toplev.cc:319
> 0xba584f gfc_free_namespace(gfc_namespace*&)
>          ../../gcc-trunk/gcc/fortran/symbol.cc:4207
> 0xba39c1 gfc_free_symbol(gfc_symbol*&)
>          ../../gcc-trunk/gcc/fortran/symbol.cc:3173
> 0xba3b89 gfc_release_symbol(gfc_symbol*&)
>          ../../gcc-trunk/gcc/fortran/symbol.cc:3216
> 0xba5339 free_sym_tree
>          ../../gcc-trunk/gcc/fortran/symbol.cc:4029
> 0xba5609 gfc_free_namespace(gfc_namespace*&)
>          ../../gcc-trunk/gcc/fortran/symbol.cc:4168
> 0xba58ef gfc_symbol_done_2()
>          ../../gcc-trunk/gcc/fortran/symbol.cc:4236
> 0xb12ec8 gfc_done_2()
>          ../../gcc-trunk/gcc/fortran/misc.cc:387
> 0xb4ac7f clean_up_modules
>          ../../gcc-trunk/gcc/fortran/parse.cc:7057
> 0xb4af02 translate_all_program_units
>          ../../gcc-trunk/gcc/fortran/parse.cc:7122
> 0xb4b735 gfc_parse_file()
>          ../../gcc-trunk/gcc/fortran/parse.cc:7413
> 0xbb626f gfc_be_parse_file
>          ../../gcc-trunk/gcc/fortran/f95-lang.cc:241
> 
> I'll stop here...
> 
Thanks. Go figure, I have no problem reproducing today.
It's PR99798 (and there is even a patch for it).

> We currently do not recover well from errors, and the prevention
> of corrupted namespaces is apparently a goal we aim at.
> 
Yes, and we are not there yet. But at least there is a sensible error 
message before the crash.

> Cheers,
> Harald
> 
>>> The patch therefore does not require any testsuite update and
>>> should not give any other surprises, so it should be very safe.
>>> The plan is also to leave the PR open for the time being.
>>>
>>> Regtesting on x86_64-pc-linux-gnu.  OK for mainline?
>>>
>>> Thanks,
>>> Harald
>>>
>>
>>
>
  
Harald Anlauf May 10, 2024, 7:48 p.m. UTC | #4
Hi Mikael,

Am 10.05.24 um 11:45 schrieb Mikael Morin:
> Le 09/05/2024 à 22:30, Harald Anlauf a écrit :
>> I'll stop here...
>>
> Thanks. Go figure, I have no problem reproducing today.
> It's PR99798 (and there is even a patch for it).

this patch has rotten a bit: the type of gfc_reluease_symbol
has changed to bool, this can be fixed.

Unfortunately, applying the patch does not remove the ICEs here...

>> We currently do not recover well from errors, and the prevention
>> of corrupted namespaces is apparently a goal we aim at.
>>
> Yes, and we are not there yet. But at least there is a sensible error
> message before the crash.

True.  But having a sensible error before ICEing does not improve
user experience either.

Are you planning to work again on PR99798?

Cheers,
Harald

>> Cheers,
>> Harald
>>
>>>> The patch therefore does not require any testsuite update and
>>>> should not give any other surprises, so it should be very safe.
>>>> The plan is also to leave the PR open for the time being.
>>>>
>>>> Regtesting on x86_64-pc-linux-gnu.  OK for mainline?
>>>>
>>>> Thanks,
>>>> Harald
>>>>
>>>
>>>
>>
>
>
  
Harald Anlauf May 10, 2024, 7:56 p.m. UTC | #5
Am 10.05.24 um 21:48 schrieb Harald Anlauf:
> Hi Mikael,
>
> Am 10.05.24 um 11:45 schrieb Mikael Morin:
>> Le 09/05/2024 à 22:30, Harald Anlauf a écrit :
>>> I'll stop here...
>>>
>> Thanks. Go figure, I have no problem reproducing today.
>> It's PR99798 (and there is even a patch for it).
>
> this patch has rotten a bit: the type of gfc_reluease_symbol
> has changed to bool, this can be fixed.
>
> Unfortunately, applying the patch does not remove the ICEs here...

Oops, I take that back!  There was an error on my side applying the
patch; and now it does fix the ICEs after correcting that hickup....

>
>>> We currently do not recover well from errors, and the prevention
>>> of corrupted namespaces is apparently a goal we aim at.
>>>
>> Yes, and we are not there yet. But at least there is a sensible error
>> message before the crash.
>
> True.  But having a sensible error before ICEing does not improve
> user experience either.
>
> Are you planning to work again on PR99798?
>
> Cheers,
> Harald
>
>>> Cheers,
>>> Harald
>>>
>>>>> The patch therefore does not require any testsuite update and
>>>>> should not give any other surprises, so it should be very safe.
>>>>> The plan is also to leave the PR open for the time being.
>>>>>
>>>>> Regtesting on x86_64-pc-linux-gnu.  OK for mainline?
>>>>>
>>>>> Thanks,
>>>>> Harald
>>>>>
>>>>
>>>>
>>>
>>
>>
>
>
  
Mikael Morin May 13, 2024, 7:25 a.m. UTC | #6
Le 10/05/2024 à 21:56, Harald Anlauf a écrit :
> Am 10.05.24 um 21:48 schrieb Harald Anlauf:
>> Hi Mikael,
>>
>> Am 10.05.24 um 11:45 schrieb Mikael Morin:
>>> Le 09/05/2024 à 22:30, Harald Anlauf a écrit :
>>>> I'll stop here...
>>>>
>>> Thanks. Go figure, I have no problem reproducing today.
>>> It's PR99798 (and there is even a patch for it).
>>
>> this patch has rotten a bit: the type of gfc_reluease_symbol
>> has changed to bool, this can be fixed.
>>
>> Unfortunately, applying the patch does not remove the ICEs here...
> 
> Oops, I take that back!  There was an error on my side applying the
> patch; and now it does fix the ICEs after correcting that hickup....
> 
Now the PR99798 patch is ready to be pushed, but I won't be available 
for a few days.  We can finish our discussion on this topic afterwards.

>>
>>>> We currently do not recover well from errors, and the prevention
>>>> of corrupted namespaces is apparently a goal we aim at.
>>>>
>>> Yes, and we are not there yet. But at least there is a sensible error
>>> message before the crash.
>>
>> True.  But having a sensible error before ICEing does not improve
>> user experience either.
>>
>> Are you planning to work again on PR99798?
>>
>> Cheers,
>> Harald
>>
>>>> Cheers,
>>>> Harald
>>>>
>>>>>> The patch therefore does not require any testsuite update and
>>>>>> should not give any other surprises, so it should be very safe.
>>>>>> The plan is also to leave the PR open for the time being.
>>>>>>
>>>>>> Regtesting on x86_64-pc-linux-gnu.  OK for mainline?
>>>>>>
>>>>>> Thanks,
>>>>>> Harald
>>>>>>
>>>>>
>>>>>
>>>>
>>>
>>>
>>
>>
>
  

Patch

From c55cb36a6ad00996b5efb33c0c5357fc5fa9919c Mon Sep 17 00:00:00 2001
From: Harald Anlauf <anlauf@gmx.de>
Date: Mon, 6 May 2024 20:57:29 +0200
Subject: [PATCH] Fortran: improve attribute conflict checking [PR93635]

gcc/fortran/ChangeLog:

	PR fortran/93635
	* symbol.cc (gfc_check_conflict): Move some attribute conflict
	checks that depend on the selected version of the Fortran standard
	so that error reporting gets more consistent.

gcc/testsuite/ChangeLog:

	PR fortran/93635
	* gfortran.dg/pr93635.f90: New test.
---
 gcc/fortran/symbol.cc                 | 30 ++++++++++++---------------
 gcc/testsuite/gfortran.dg/pr93635.f90 | 19 +++++++++++++++++
 2 files changed, 32 insertions(+), 17 deletions(-)
 create mode 100644 gcc/testsuite/gfortran.dg/pr93635.f90

diff --git a/gcc/fortran/symbol.cc b/gcc/fortran/symbol.cc
index 8f7deac1d1e..ed17291c53e 100644
--- a/gcc/fortran/symbol.cc
+++ b/gcc/fortran/symbol.cc
@@ -459,22 +459,6 @@  gfc_check_conflict (symbol_attribute *attr, const char *name, locus *where)
   if (where == NULL)
     where = &gfc_current_locus;

-  if (attr->pointer && attr->intent != INTENT_UNKNOWN)
-    {
-      a1 = pointer;
-      a2 = intent;
-      standard = GFC_STD_F2003;
-      goto conflict_std;
-    }
-
-  if (attr->in_namelist && (attr->allocatable || attr->pointer))
-    {
-      a1 = in_namelist;
-      a2 = attr->allocatable ? allocatable : pointer;
-      standard = GFC_STD_F2003;
-      goto conflict_std;
-    }
-
   /* Check for attributes not allowed in a BLOCK DATA.  */
   if (gfc_current_state () == COMP_BLOCK_DATA)
     {
@@ -579,10 +563,12 @@  gfc_check_conflict (symbol_attribute *attr, const char *name, locus *where)
     return false;

   conf (allocatable, pointer);
+
+  /* Moving these checks past the function/subroutine conflict check may
+     cause trouble with minor variations of testcase pr87907.f90.  */
   conf_std (allocatable, dummy, GFC_STD_F2003);
   conf_std (allocatable, function, GFC_STD_F2003);
   conf_std (allocatable, result, GFC_STD_F2003);
-  conf_std (elemental, recursive, GFC_STD_F2018);

   conf (in_common, dummy);
   conf (in_common, allocatable);
@@ -911,6 +897,16 @@  gfc_check_conflict (symbol_attribute *attr, const char *name, locus *where)
       break;
     }

+  /* Conflict checks depending on the selected version of the Fortran
+     standard are preferably applied after standard-independent ones, so
+     that one gets more consistent error reporting and recovery.  */
+  if (attr->pointer && attr->intent != INTENT_UNKNOWN)
+    conf_std (pointer, intent, GFC_STD_F2003);
+
+  conf_std (in_namelist, allocatable, GFC_STD_F2003);
+  conf_std (in_namelist, pointer, GFC_STD_F2003);
+  conf_std (elemental, recursive, GFC_STD_F2018);
+
   return true;

 conflict:
diff --git a/gcc/testsuite/gfortran.dg/pr93635.f90 b/gcc/testsuite/gfortran.dg/pr93635.f90
new file mode 100644
index 00000000000..4ef33fecf2b
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/pr93635.f90
@@ -0,0 +1,19 @@ 
+! { dg-do compile }
+! PR fortran/93635
+!
+! Test that some attribute conflicts are properly diagnosed
+
+program p
+  implicit none
+  character(len=:),allocatable :: r,s
+  namelist /args/ r,s
+  equivalence(r,s) ! { dg-error "EQUIVALENCE attribute conflicts with ALLOCATABLE" }
+  allocate(character(len=1024) :: r)
+end
+
+subroutine sub (p, q)
+  implicit none
+  real, pointer, intent(inout) :: p(:), q(:)
+  namelist /nml/ p,q
+  equivalence(p,q) ! { dg-error "EQUIVALENCE attribute conflicts with DUMMY" }
+end
--
2.35.3