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