public inbox for fortran@gcc.gnu.org
 help / color / mirror / Atom feed
From: Harald Anlauf <anlauf@gmx.de>
To: Bernhard Reutner-Fischer <rep.dot.nop@gmail.com>
Cc: Harald Anlauf via Fortran <fortran@gcc.gnu.org>, gcc-patches@gcc.gnu.org
Subject: Re: [PATCH] Fortran: Mark internal symbols as artificial [PR88009,PR68800]
Date: Mon, 29 Jan 2024 22:06:04 +0100	[thread overview]
Message-ID: <a33f23b6-8ef7-4ea0-8788-a0a2a92ab579@gmx.de> (raw)
In-Reply-To: <20240129214509.590a7fa0@nbbrfq.loc>

Am 29.01.24 um 21:45 schrieb Bernhard Reutner-Fischer:
> On Wed, 17 Nov 2021 21:32:14 +0100
> Harald Anlauf <anlauf@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 <fortran@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?


  reply	other threads:[~2024-01-29 21:06 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-14 22:17 Bernhard Reutner-Fischer
2021-11-16 20:46 ` Harald Anlauf
2021-11-17  8:12   ` Bernhard Reutner-Fischer
2021-11-17 20:32     ` Harald Anlauf
2024-01-29 20:45       ` Bernhard Reutner-Fischer
2024-01-29 21:06         ` Harald Anlauf [this message]
2024-01-29 22:18           ` rep.dot.nop

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=a33f23b6-8ef7-4ea0-8788-a0a2a92ab579@gmx.de \
    --to=anlauf@gmx.de \
    --cc=fortran@gcc.gnu.org \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=rep.dot.nop@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).