On 29 January 2024 22:06:04 CET, Harald Anlauf <anl...@gmx.de> wrote:
>Am 29.01.24 um 21:45 schrieb Bernhard Reutner-Fischer:
>> On Wed, 17 Nov 2021 21:32:14 +0100
>> Harald Anlauf <anl...@gmx.de> wrote:
>>
>>> Do you have testcases/reproducers demonstrating that the patch actually
>>> fixes the issues you're describing?
>>
>> I believe that marking artificial symbols as such is obvious and i did
>> use the existing tests to verify that the changes do not regress but
>> behave as intended. I did check that the memory leak in
>> gfc_find_derived_vtab is fixed with the patch.
>>
>> Ok for stage 1 if the rebased regression test passes?
>>
>> thanks
>>
>>>
>>> Am 17.11.21 um 09:12 schrieb Bernhard Reutner-Fischer via Gcc-patches:
>>>> On Tue, 16 Nov 2021 21:46:32 +0100
>>>> Harald Anlauf via Fortran <fort...@gcc.gnu.org> wrote:
>>>>
>>>>> Hi Bernhard,
>>>>>
>>>>> I'm trying to understand your patch. What does it really try to solve?
>>>>
>>>> Compiler generated symbols should be marked artificial.
>>>> The fix for PR88009 ( f8add009ce300f24b75e9c2e2cc5dd944a020c28 ,
>>>> r9-5194 ) added artificial just to the _final component and left out all
>>>> the rest.
>>>> Note that the majority of compiler generated symbols in class.c
>>>> already had artificial set properly.
>>>> The proposed patch amends the other generated symbols to be marked
>>>> artificial, too.
>>>>
>>>> The other parts fix memory leaks.
>>>>
>>>>>
>>>>> PR88009 is closed and seems to have nothing to do with this.
>>>>
>>>> Well it marked only _final as artificial and forgot to adjust the
>>>> others as well.
>>>> We can remove the reference to PR88009 if you prefer?
>>>>
>>>> thanks!
>>>>>
>>>>> Harald
>>>>>
>>>>> Am 14.11.21 um 23:17 schrieb Bernhard Reutner-Fischer via Fortran:
>>>>>> Hi!
>>>>>>
>>>>>> Amend fix for PR88009 to mark all these class components as artificial.
>>>>>>
>>>>>> gcc/fortran/ChangeLog:
>>>>>>
>>>>>> * class.c (gfc_build_class_symbol,
>>>>>> generate_finalization_wrapper,
>>>>>> (gfc_find_derived_vtab, find_intrinsic_vtab): Use stringpool
>>>>>> for
>>>>>> names. Mark internal symbols as artificial.
>>>>>> * decl.c (gfc_match_decl_type_spec, gfc_match_end): Fix
>>>>>> indentation.
>>>>>> (gfc_match_derived_decl): Fix indentation. Check extension
>>>>>> level
>>>>>> before incrementing refs counter.
>>>>>> * parse.c (parse_derived): Fix style.
>>>>>> * resolve.c (resolve_global_procedure): Likewise.
>>>>>> * symbol.c (gfc_check_conflict): Do not ignore artificial
>>>>>> symbols.
>>>>>> (gfc_add_flavor): Reorder condition, cheapest first.
>>>>>> (gfc_new_symbol, gfc_get_sym_tree,
>>>>>> generate_isocbinding_symbol): Fix style.
>>>>>> * trans-expr.c (gfc_trans_subcomponent_assign): Remove
>>>>>> restriction on !artificial.
>>>>>> * match.c (gfc_match_equivalence): Special-case CLASS_DATA for
>>>>>> warnings.
>>>>>>
>>>>>> ---
>>>>>> gfc_match_equivalence(), too, should not bail-out early on the first
>>>>>> error but should diagnose all errors. I.e. not goto cleanup but set
>>>>>> err=true and continue in order to diagnose all constraints of a
>>>>>> statement. Maybe Sandra or somebody else will eventually find time to
>>>>>> tweak that.
>>>>>>
>>>>>> I think it also plugs a very minor leak of name in gfc_find_derived_vtab
>>>>>> so i also tagged it [PR68800]. At least that was the initial
>>>>>> motiviation to look at that spot.
>>>>>> We were doing
>>>>>> - name = xasprintf ("__vtab_%s", tname);
>>>>>> ...
>>>>>> gfc_set_sym_referenced (vtab);
>>>>>> - name = xasprintf ("__vtype_%s", tname);
>>>>>>
>>>>>> Bootstrapped and regtested without regressions on x86_64-unknown-linux.
>>>>>> Ok for trunk?
>>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>>
>>
>>
>
>Can you please post the patch here so that we can review it?
>
I'm very sorry that I missed to provide an explicit reference, the initial
patch was submitted here:
https://inbox.sourceware.org/fortran/20211114231748.376086cd@nbbrfq/
But I will follow-up with a rebased, tested patch ASAP during a weekend or
vacation.
But just to give context, that's what I was talking about..
thanks
PS: IMHO a strcmp(..,"_data") is inappropriate for you should check
attr.artificial and the path exploder to give hints should ideally deal with
this transparently -- IMHO
