imcc optimization interferes with lexical subs

[pmichaud]

Summary: IMCC's optimization of directly linking sub calls to identically-named subs in the namespace interferes with lexical sub calls.

Background: In PIR, a call of the form 'foo'(args) is usually translated by IMCC into a sequence like:

    $P0 = find_sub_not_null 'foo'
    $P0(args)

The find_sub_not_null opcode is a specialized form of find_name; i.e., it searches the current lexical, package, and global scopes (in that order) for the given name and returns the associated PMC. If no such PMC is found, it throws a "'foo' not found" exception.

However, if IMCC detects that the source it's compiling already has an identically-named sub in the current namespace, it bypasses the find_sub_not_null opcode and creates a call directly to that sub. While this works much of the time, it fails if there's a lexical name in scope, as the lexical sub should take priority over the package-scoped sub.

Here's an example illustrating the problem:

$ cat z.pir
.sub 'main'
    .const 'Sub' $P0 = 'lexfoo'
    .lex 'foo1', $P0
    .lex 'foo2', $P0

    'foo1'()
    'foo2'()
.end

.sub 'lexfoo'
    say 'right foo'
.end

.sub 'foo2'
    say 'wrong foo'
.end

$ ./parrot z.pir
right foo
wrong foo
$

Using the trace option to Parrot verifies that different code is being generated for the calls to 'foo1'() and 'foo2'().

I can envision a number of possible solutions to the problem:

(1) Disable IMCC's optimization completely, and have all calls go through 'find_sub_not_null'.

(2) Disable the optimization for subs that have lexical pads or :outer flags.

(3) Leave things as they are, and have code generators (e.g. PCT) try to detect when this situation might occur and explicitly code around it. (Note that there may be some situations in which this is not generally possible without a great deal of pessimisation or run-time scope analysis on the part of the code generator. Also, "coding around" it may cause even further complications down the road, as languages come to rely on specific behavior of the workaround.)

For any of the above there might be a question of needing a deprecation cycle for the existing behavior; countering this may be (a) the existing behavior might be considered a bug to be fixed as opposed to a feature to be retained, and (b) it's very unlikely to affect existing code significantly, as very little code outside of PCT-generated stuff uses lexicals, much less lexical subs.

As to severity, this isn't hypothetical -- Jonathan encountered this problem directly while implementing lexical subs in Rakudo (and afaik we don't have a workaround implemented yet, pending resolution of this ticket).

Comments welcomed,

Pm

[allison]

I'd like to see some benchmarks before and after, but I'd be inclined toward option 1.

[chromatic]

This one is tricky.

r40270 appears to do the right thing. I chose option #4, which is to disable cached static lookups when using a lexical to call a sub.