[PATCH] Fortran: improve attribute conflict checking [PR93635]

[PATCH] Fortran: improve attribute conflict checking [PR93635]

[Harald Anlauf]

[-- Attachment #1: Type: text/plain, Size: 1461 bytes --]

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


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: pr93635.diff --]
[-- Type: text/x-patch, Size: 3631 bytes --]

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

Re: [PATCH] Fortran: improve attribute conflict checking [PR93635]

[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?

> 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
> 

Re: [PATCH] Fortran: improve attribute conflict checking [PR93635]

[Harald Anlauf]

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
>>
>
>

Re: [PATCH] Fortran: improve attribute conflict checking [PR93635]

[Harald Anlauf]

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
>>
> 
> 

Re: [PATCH] Fortran: improve attribute conflict checking [PR93635]

[Mikael Morin]

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
>>>
>>
>>
> 

Re: [PATCH] Fortran: improve attribute conflict checking [PR93635]

[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...

>> 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
>>>>
>>>
>>>
>>
>
>

Re: [PATCH] Fortran: improve attribute conflict checking [PR93635]

[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...

>> 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
>>>>
>>>
>>>
>>
> 
> 

Re: [PATCH] Fortran: improve attribute conflict checking [PR93635]

[Harald Anlauf]

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
>>>>>
>>>>
>>>>
>>>
>>
>>
>
>

Re: [PATCH] Fortran: improve attribute conflict checking [PR93635]

[Harald Anlauf]

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
>>>>>
>>>>
>>>>
>>>
>>
>>
> 
> 

Re: [PATCH] Fortran: improve attribute conflict checking [PR93635]

[Mikael Morin]

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
>>>>>>
>>>>>
>>>>>
>>>>
>>>
>>>
>>
>>
> 

Re: [PATCH] Fortran: improve attribute conflict checking [PR93635]

[Mikael Morin]

Le 13/05/2024 à 09:25, Mikael Morin a écrit :
> 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.
> 
Hello,

I'm coming back to this.
I think either one of Steve's patch or your variant in the PR is a 
better fix for the ICE as a first step; they seem less fragile at least.
Then we can look at a possible reordering of conflict checks as with the 
patch you originally submitted in this thread.

Mikael

[PATCH, v2] Fortran: improve attribute conflict checking [PR93635]

[Harald Anlauf]

[-- Attachment #1: Type: text/plain, Size: 1369 bytes --]

Hi Mikael,

On 5/23/24 09:49, Mikael Morin wrote:
> Le 13/05/2024 à 09:25, Mikael Morin a écrit :
>> 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.
>>
> Hello,
>
> I'm coming back to this.
> I think either one of Steve's patch or your variant in the PR is a
> better fix for the ICE as a first step; they seem less fragile at least.
> Then we can look at a possible reordering of conflict checks as with the
> patch you originally submitted in this thread.

like the attached variant?

Harald

> Mikael
>

[-- Attachment #2: pr93635-v2.diff --]
[-- Type: text/x-patch, Size: 6456 bytes --]

From 68d73e6e2efa692afff10ea16eafb88236cbe69c Mon Sep 17 00:00:00 2001
From: Harald Anlauf <anlauf@gmx.de>
Date: Thu, 23 May 2024 21:13:00 +0200
Subject: [PATCH] Fortran: improve attribute conflict checking [PR93635]

gcc/fortran/ChangeLog:

	PR fortran/93635
	* symbol.cc (conflict_std): Helper function for reporting attribute
	conflicts depending on the Fortran standard version.
	(conf_std): Helper macro for checking standard-dependent conflicts.
	(gfc_check_conflict): Use it.

gcc/testsuite/ChangeLog:

	PR fortran/93635
	* gfortran.dg/c-interop/c1255-2.f90: Adjust pattern.
	* gfortran.dg/pr87907.f90: Likewise.
	* gfortran.dg/pr93635.f90: New test.

Co-authored-by: Steven G. Kargl <kargl@gcc.gnu.org>
---
 gcc/fortran/symbol.cc                         | 63 +++++++++----------
 .../gfortran.dg/c-interop/c1255-2.f90         |  4 +-
 gcc/testsuite/gfortran.dg/pr87907.f90         |  8 ++-
 gcc/testsuite/gfortran.dg/pr93635.f90         | 19 ++++++
 4 files changed, 54 insertions(+), 40 deletions(-)
 create mode 100644 gcc/testsuite/gfortran.dg/pr93635.f90

diff --git a/gcc/fortran/symbol.cc b/gcc/fortran/symbol.cc
index 0a1646def67..5db3c887127 100644
--- a/gcc/fortran/symbol.cc
+++ b/gcc/fortran/symbol.cc
@@ -407,18 +407,36 @@ gfc_check_function_type (gfc_namespace *ns)
 
 /******************** Symbol attribute stuff *********************/
 
+/* Older standards produced conflicts for some attributes that are allowed
+   in newer standards.  Check for the conflict and issue an error depending
+   on the standard in play.  */
+
+static bool
+conflict_std (int standard, const char *a1, const char *a2, const char *name,
+	      locus *where)
+{
+  if (name == NULL)
+    {
+      return gfc_notify_std (standard, "%s attribute conflicts "
+			     "with %s attribute at %L", a1, a2,
+			     where);
+    }
+  else
+    {
+      return gfc_notify_std (standard, "%s attribute conflicts "
+			     "with %s attribute in %qs at %L",
+			     a1, a2, name, where);
+    }
+}
+
 /* This is a generic conflict-checker.  We do this to avoid having a
    single conflict in two places.  */
 
 #define conf(a, b) if (attr->a && attr->b) { a1 = a; a2 = b; goto conflict; }
 #define conf2(a) if (attr->a) { a2 = a; goto conflict; }
-#define conf_std(a, b, std) if (attr->a && attr->b)\
-                              {\
-                                a1 = a;\
-                                a2 = b;\
-                                standard = std;\
-                                goto conflict_std;\
-                              }
+#define conf_std(a, b, std) if (attr->a && attr->b \
+				&& !conflict_std (std, a, b, name, where)) \
+				return false;
 
 bool
 gfc_check_conflict (symbol_attribute *attr, const char *name, locus *where)
@@ -451,7 +469,6 @@ gfc_check_conflict (symbol_attribute *attr, const char *name, locus *where)
 						"OACC DECLARE DEVICE_RESIDENT";
 
   const char *a1, *a2;
-  int standard;
 
   if (attr->artificial)
     return true;
@@ -460,20 +477,10 @@ gfc_check_conflict (symbol_attribute *attr, const char *name, locus *where)
     where = &gfc_current_locus;
 
   if (attr->pointer && attr->intent != INTENT_UNKNOWN)
-    {
-      a1 = pointer;
-      a2 = intent;
-      standard = GFC_STD_F2003;
-      goto conflict_std;
-    }
+    conf_std (pointer, intent, GFC_STD_F2003);
 
-  if (attr->in_namelist && (attr->allocatable || attr->pointer))
-    {
-      a1 = in_namelist;
-      a2 = attr->allocatable ? allocatable : pointer;
-      standard = GFC_STD_F2003;
-      goto conflict_std;
-    }
+  conf_std (in_namelist, allocatable, GFC_STD_F2003);
+  conf_std (in_namelist, pointer, GFC_STD_F2003);
 
   /* Check for attributes not allowed in a BLOCK DATA.  */
   if (gfc_current_state () == COMP_BLOCK_DATA)
@@ -922,20 +929,6 @@ conflict:
 	       a1, a2, name, where);
 
   return false;
-
-conflict_std:
-  if (name == NULL)
-    {
-      return gfc_notify_std (standard, "%s attribute conflicts "
-                             "with %s attribute at %L", a1, a2,
-                             where);
-    }
-  else
-    {
-      return gfc_notify_std (standard, "%s attribute conflicts "
-			     "with %s attribute in %qs at %L",
-                             a1, a2, name, where);
-    }
 }
 
 #undef conf
diff --git a/gcc/testsuite/gfortran.dg/c-interop/c1255-2.f90 b/gcc/testsuite/gfortran.dg/c-interop/c1255-2.f90
index 0e5505a0183..feed2e7645f 100644
--- a/gcc/testsuite/gfortran.dg/c-interop/c1255-2.f90
+++ b/gcc/testsuite/gfortran.dg/c-interop/c1255-2.f90
@@ -92,12 +92,12 @@ module m2
     end function
 
     ! function result is a type that is not interoperable
-    function g (x) bind (c)  ! { dg-error "BIND\\(C\\)" }
+    function g (x) bind (c)  ! { dg-error "has no IMPLICIT type" }
       use ISO_C_BINDING
       use m1
       implicit none
       integer(C_INT) :: x
-      integer(C_INT), allocatable :: g
+      integer(C_INT), allocatable :: g  ! { dg-error "BIND\\(C\\) attribute conflicts with ALLOCATABLE" }
     end function
 
   end interface
diff --git a/gcc/testsuite/gfortran.dg/pr87907.f90 b/gcc/testsuite/gfortran.dg/pr87907.f90
index 0fe4e5090d2..5c2acaf9b7f 100644
--- a/gcc/testsuite/gfortran.dg/pr87907.f90
+++ b/gcc/testsuite/gfortran.dg/pr87907.f90
@@ -12,12 +12,14 @@ end
 
 submodule(m) m2
    contains
-      subroutine g(x)   ! { dg-error "mismatch in argument" }
+      subroutine g(x) ! { dg-error "FUNCTION attribute conflicts with SUBROUTINE" }
       end
 end
 
 program p
-   use m                ! { dg-error "has a type" }
+   use m
    integer :: x = 3
-   call g(x)            ! { dg-error "which is not consistent with" }
+   call g(x)
 end
+
+! { dg-prune-output "Two main PROGRAMs" }
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

[PATCH, v2] Fortran: improve attribute conflict checking [PR93635]

[Harald Anlauf]

[-- Attachment #1: Type: text/plain, Size: 1334 bytes --]

Hi Mikael,

On 5/23/24 09:49, Mikael Morin wrote:
> Le 13/05/2024 à 09:25, Mikael Morin a écrit :
>> 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.
>>
> Hello,
> 
> I'm coming back to this.
> I think either one of Steve's patch or your variant in the PR is a 
> better fix for the ICE as a first step; they seem less fragile at least.
> Then we can look at a possible reordering of conflict checks as with the 
> patch you originally submitted in this thread.

like the attached variant?

Harald

> Mikael
> 

[-- Attachment #2: pr93635-v2.diff --]
[-- Type: text/x-patch, Size: 6456 bytes --]

From 68d73e6e2efa692afff10ea16eafb88236cbe69c Mon Sep 17 00:00:00 2001
From: Harald Anlauf <anlauf@gmx.de>
Date: Thu, 23 May 2024 21:13:00 +0200
Subject: [PATCH] Fortran: improve attribute conflict checking [PR93635]

gcc/fortran/ChangeLog:

	PR fortran/93635
	* symbol.cc (conflict_std): Helper function for reporting attribute
	conflicts depending on the Fortran standard version.
	(conf_std): Helper macro for checking standard-dependent conflicts.
	(gfc_check_conflict): Use it.

gcc/testsuite/ChangeLog:

	PR fortran/93635
	* gfortran.dg/c-interop/c1255-2.f90: Adjust pattern.
	* gfortran.dg/pr87907.f90: Likewise.
	* gfortran.dg/pr93635.f90: New test.

Co-authored-by: Steven G. Kargl <kargl@gcc.gnu.org>
---
 gcc/fortran/symbol.cc                         | 63 +++++++++----------
 .../gfortran.dg/c-interop/c1255-2.f90         |  4 +-
 gcc/testsuite/gfortran.dg/pr87907.f90         |  8 ++-
 gcc/testsuite/gfortran.dg/pr93635.f90         | 19 ++++++
 4 files changed, 54 insertions(+), 40 deletions(-)
 create mode 100644 gcc/testsuite/gfortran.dg/pr93635.f90

diff --git a/gcc/fortran/symbol.cc b/gcc/fortran/symbol.cc
index 0a1646def67..5db3c887127 100644
--- a/gcc/fortran/symbol.cc
+++ b/gcc/fortran/symbol.cc
@@ -407,18 +407,36 @@ gfc_check_function_type (gfc_namespace *ns)
 
 /******************** Symbol attribute stuff *********************/
 
+/* Older standards produced conflicts for some attributes that are allowed
+   in newer standards.  Check for the conflict and issue an error depending
+   on the standard in play.  */
+
+static bool
+conflict_std (int standard, const char *a1, const char *a2, const char *name,
+	      locus *where)
+{
+  if (name == NULL)
+    {
+      return gfc_notify_std (standard, "%s attribute conflicts "
+			     "with %s attribute at %L", a1, a2,
+			     where);
+    }
+  else
+    {
+      return gfc_notify_std (standard, "%s attribute conflicts "
+			     "with %s attribute in %qs at %L",
+			     a1, a2, name, where);
+    }
+}
+
 /* This is a generic conflict-checker.  We do this to avoid having a
    single conflict in two places.  */
 
 #define conf(a, b) if (attr->a && attr->b) { a1 = a; a2 = b; goto conflict; }
 #define conf2(a) if (attr->a) { a2 = a; goto conflict; }
-#define conf_std(a, b, std) if (attr->a && attr->b)\
-                              {\
-                                a1 = a;\
-                                a2 = b;\
-                                standard = std;\
-                                goto conflict_std;\
-                              }
+#define conf_std(a, b, std) if (attr->a && attr->b \
+				&& !conflict_std (std, a, b, name, where)) \
+				return false;
 
 bool
 gfc_check_conflict (symbol_attribute *attr, const char *name, locus *where)
@@ -451,7 +469,6 @@ gfc_check_conflict (symbol_attribute *attr, const char *name, locus *where)
 						"OACC DECLARE DEVICE_RESIDENT";
 
   const char *a1, *a2;
-  int standard;
 
   if (attr->artificial)
     return true;
@@ -460,20 +477,10 @@ gfc_check_conflict (symbol_attribute *attr, const char *name, locus *where)
     where = &gfc_current_locus;
 
   if (attr->pointer && attr->intent != INTENT_UNKNOWN)
-    {
-      a1 = pointer;
-      a2 = intent;
-      standard = GFC_STD_F2003;
-      goto conflict_std;
-    }
+    conf_std (pointer, intent, GFC_STD_F2003);
 
-  if (attr->in_namelist && (attr->allocatable || attr->pointer))
-    {
-      a1 = in_namelist;
-      a2 = attr->allocatable ? allocatable : pointer;
-      standard = GFC_STD_F2003;
-      goto conflict_std;
-    }
+  conf_std (in_namelist, allocatable, GFC_STD_F2003);
+  conf_std (in_namelist, pointer, GFC_STD_F2003);
 
   /* Check for attributes not allowed in a BLOCK DATA.  */
   if (gfc_current_state () == COMP_BLOCK_DATA)
@@ -922,20 +929,6 @@ conflict:
 	       a1, a2, name, where);
 
   return false;
-
-conflict_std:
-  if (name == NULL)
-    {
-      return gfc_notify_std (standard, "%s attribute conflicts "
-                             "with %s attribute at %L", a1, a2,
-                             where);
-    }
-  else
-    {
-      return gfc_notify_std (standard, "%s attribute conflicts "
-			     "with %s attribute in %qs at %L",
-                             a1, a2, name, where);
-    }
 }
 
 #undef conf
diff --git a/gcc/testsuite/gfortran.dg/c-interop/c1255-2.f90 b/gcc/testsuite/gfortran.dg/c-interop/c1255-2.f90
index 0e5505a0183..feed2e7645f 100644
--- a/gcc/testsuite/gfortran.dg/c-interop/c1255-2.f90
+++ b/gcc/testsuite/gfortran.dg/c-interop/c1255-2.f90
@@ -92,12 +92,12 @@ module m2
     end function
 
     ! function result is a type that is not interoperable
-    function g (x) bind (c)  ! { dg-error "BIND\\(C\\)" }
+    function g (x) bind (c)  ! { dg-error "has no IMPLICIT type" }
       use ISO_C_BINDING
       use m1
       implicit none
       integer(C_INT) :: x
-      integer(C_INT), allocatable :: g
+      integer(C_INT), allocatable :: g  ! { dg-error "BIND\\(C\\) attribute conflicts with ALLOCATABLE" }
     end function
 
   end interface
diff --git a/gcc/testsuite/gfortran.dg/pr87907.f90 b/gcc/testsuite/gfortran.dg/pr87907.f90
index 0fe4e5090d2..5c2acaf9b7f 100644
--- a/gcc/testsuite/gfortran.dg/pr87907.f90
+++ b/gcc/testsuite/gfortran.dg/pr87907.f90
@@ -12,12 +12,14 @@ end
 
 submodule(m) m2
    contains
-      subroutine g(x)   ! { dg-error "mismatch in argument" }
+      subroutine g(x) ! { dg-error "FUNCTION attribute conflicts with SUBROUTINE" }
       end
 end
 
 program p
-   use m                ! { dg-error "has a type" }
+   use m
    integer :: x = 3
-   call g(x)            ! { dg-error "which is not consistent with" }
+   call g(x)
 end
+
+! { dg-prune-output "Two main PROGRAMs" }
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

Re: [PATCH] Fortran: improve attribute conflict checking [PR93635]

[Mikael Morin]

Le 23/05/2024 à 09:49, Mikael Morin a écrit :
> Le 13/05/2024 à 09:25, Mikael Morin a écrit :
>> 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.
>>
> Hello,
> 
> I'm coming back to this.
> I think either one of Steve's patch or your variant in the PR is a 
> better fix for the ICE as a first step; they seem less fragile at least.
> Then we can look at a possible reordering of conflict checks as with the 
> patch you originally submitted in this thread.
> 
Replying to myself...
It's not a great plan if we want to avoid unnecessary churn in the 
testsuite.

Re: [PATCH, v2] Fortran: improve attribute conflict checking [PR93635]

[Mikael Morin]

Le 23/05/2024 à 21:15, Harald Anlauf a écrit :
> Hi Mikael,
> 
> On 5/23/24 09:49, Mikael Morin wrote:
>> Le 13/05/2024 à 09:25, Mikael Morin a écrit :
>>> 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.
>>>
>> Hello,
>>
>> I'm coming back to this.
>> I think either one of Steve's patch or your variant in the PR is a
>> better fix for the ICE as a first step; they seem less fragile at least.
>> Then we can look at a possible reordering of conflict checks as with the
>> patch you originally submitted in this thread.
> 
> like the attached variant?
> 
Yes.  The churn in the testsuite is actually not that bad.
OK for master, thanks for the patch.
I wouldn't push for backporting, but if you feel like doing it, it seems 
safe enough (depending on my own backport for PR99798 of course).

Regarding the conflict check reordering, I'm tempted to just drop it at 
this point, or do you think it remains worth it?

Re: [PATCH, v2] Fortran: improve attribute conflict checking [PR93635]

[Harald Anlauf]

Hi Mikael,

On 5/24/24 20:17, Mikael Morin wrote:
> Le 23/05/2024 à 21:15, Harald Anlauf a écrit :
>> Hi Mikael,
>>
>> On 5/23/24 09:49, Mikael Morin wrote:
>>> Le 13/05/2024 à 09:25, Mikael Morin a écrit :
>>>> 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.
>>>>
>>> Hello,
>>>
>>> I'm coming back to this.
>>> I think either one of Steve's patch or your variant in the PR is a
>>> better fix for the ICE as a first step; they seem less fragile at least.
>>> Then we can look at a possible reordering of conflict checks as with the
>>> patch you originally submitted in this thread.
>>
>> like the attached variant?
>>
> Yes.  The churn in the testsuite is actually not that bad.
> OK for master, thanks for the patch.

thanks, will do.

> I wouldn't push for backporting, but if you feel like doing it, it seems 
> safe enough (depending on my own backport for PR99798 of course).

There's no pressing need.  I'll mark the patch as backportable
with dependency in my own list, in case the question comes up.

> Regarding the conflict check reordering, I'm tempted to just drop it at 
> this point, or do you think it remains worth it?

I don't really have a showcase where this would bring a benefit now,
so I'm dropping this idea.

There are issues where specifying a standard version changes
the error recovery path (or rather lead to an ICE), but as some
of these are due to emitting an error during parsing instead of
during resolution, my suggestion does not help there.

If you look for an example: this one is taken from pr101281

subroutine a3pr (xn) bind(C)
   character(len=n), pointer :: xn(..)
end

vs.

subroutine a3pr (xn) bind(C)
   character(len=n), pointer :: xn
   dimension :: xn(..)
end

The first one gives lots of invalid reads in valgrind with -std=f2008,
or ICEs, while the second does not.

Thanks,
Harald

Re: [PATCH, v2] Fortran: improve attribute conflict checking [PR93635]

[Harald Anlauf]

Hi Mikael,

On 5/24/24 20:17, Mikael Morin wrote:
> Le 23/05/2024 à 21:15, Harald Anlauf a écrit :
>> Hi Mikael,
>>
>> On 5/23/24 09:49, Mikael Morin wrote:
>>> Le 13/05/2024 à 09:25, Mikael Morin a écrit :
>>>> 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.
>>>>
>>> Hello,
>>>
>>> I'm coming back to this.
>>> I think either one of Steve's patch or your variant in the PR is a
>>> better fix for the ICE as a first step; they seem less fragile at least.
>>> Then we can look at a possible reordering of conflict checks as with the
>>> patch you originally submitted in this thread.
>>
>> like the attached variant?
>>
> Yes.  The churn in the testsuite is actually not that bad.
> OK for master, thanks for the patch.

thanks, will do.

> I wouldn't push for backporting, but if you feel like doing it, it seems
> safe enough (depending on my own backport for PR99798 of course).

There's no pressing need.  I'll mark the patch as backportable
with dependency in my own list, in case the question comes up.

> Regarding the conflict check reordering, I'm tempted to just drop it at
> this point, or do you think it remains worth it?

I don't really have a showcase where this would bring a benefit now,
so I'm dropping this idea.

There are issues where specifying a standard version changes
the error recovery path (or rather lead to an ICE), but as some
of these are due to emitting an error during parsing instead of
during resolution, my suggestion does not help there.

If you look for an example: this one is taken from pr101281

subroutine a3pr (xn) bind(C)
   character(len=n), pointer :: xn(..)
end

vs.

subroutine a3pr (xn) bind(C)
   character(len=n), pointer :: xn
   dimension :: xn(..)
end

The first one gives lots of invalid reads in valgrind with -std=f2008,
or ICEs, while the second does not.

Thanks,
Harald