This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [Patch, fortran] PR fortran/42769 typebound call resolved incorrectly.


Hi Mikael,

> here is a fix for PR42769, where a type bound call was resolved statically
> into a normal procedure call, but using the local procedure (with the same
> name) instead of the original (and correct) one.
> It turned out to not be OOP specific, and be a pure module loading problem
> in the way we associate a symbol number in the module file with the
> corresponding symbol pointer once module is loaded.
>
> The first module.c hunk fixes comment #14 in the PR. Here, p == NULL, which
> means that the user didn't ask to load the symbol (USE,ONLY:...), so we can
> just continue to the next one.  However, as an optimization, we try to find
> the symbol in the current name space and associate the pointer info (mapping
> module's integer <-> symbol pointer) with it if found to avoid reloading it
> from the module file if needed.
> The problem was there was no check that the symbol is really the same, not
> another symbol with the same name.  Fixed by checking symbol's name and
> module name too.  The original code was only reusing the symtree; I took the
> opportunity to reuse the symbol as well, and the test suite didn't complain
> :-).
>
> The second module.c hunk fixes comment #24 in the PR.  Here, p != NULL: the
> symbol will have to be loaded.  Before creating a new symbol and a new
> symtree and inserting them in the current name space, we check whether a
> symtree of the same name exists in the current name space. If it exists and
> is not ambiguous, it is either the same one as the one from the module, or
> both are generic so we can reuse the existing symbol in both cases.
> However, if the symtree in the name space is ambiguous we shall not reuse it
> as it is different than the one to be loaded. Thus fixed by not reusing it
> in that case, and letting a unique non ambiguous name being automatically
> generated later.
>
> The resolve.c hunk fixes comment #34 in the PR.  This one is not a module
> problem, but let's fix all cases in one go.  Resolve_call reloads the call's
> procedure symbol to take into account contained procedure which were not
> known at parse time.  However, it uses the symbol's name (original name) for
> lookup instead of the symtree's (local name).  The resolve.c hunk changes
> that.
>
> The tests are adapted from comment 14, 24, 34 from pr42769, and the original
> test cases from pr45836 and pr45900.
> Regression tested on x86_64-unknown-linux-gnu. OK for trunk?

thanks for taking care of this rather long-standing bug. I was
convinced that it is a module-loading problem, but couldn't really
nail it down.

I think your patch is indeed ok for trunk.


> 4.7?
> 4.6?

Since it's not a regression, it does not necessarily need to be
backported. However, the sort of wrong-code behavior seen in comment
#24 is severe enough that it might justify backporting. I leave that
up to you.

Cheers,
Janus


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]