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

[Harald Anlauf <anlauf@gmx.de>]

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