Scope-related mishap in 'my' declaration in quasi

[masak]

While testing out #151 (which remains closed), I found the following:

$ bin/007 -e='macro moo(x) { return quasi { (sub() { my y = {{{x}}} })() }; }; say(moo(42))'
Variable 'y' is not declared
  in method find-pad at /home/masak/ours/007/lib/_007/Runtime.pm (_007::Runtime) line 83
  in method put-var at /home/masak/ours/007/lib/_007/Runtime.pm (_007::Runtime) line 104
  in method put-value at /home/masak/ours/007/lib/_007/Q.pm (_007::Q) line 123
  in method run at /home/masak/ours/007/lib/_007/Q.pm (_007::Q) line 521
  [...]

My guess is that something gets out of kilter with the scopes of sub terms. (The rest of this paragraph was removed since it consisted entirely of various ways to phrase "with such an easy-to-exhibit bug, how hard can it be to fix?". As everyone in IT knows, actually uttering this phrase is a surefire way to curse yourself to horrible, nigh-unfixable bugs.)

[masak]

Found a simpler case. The anon sub was not necessary:

$ bin/007 -e='macro moo(a) { return quasi { my x = {{{a}}} } }; moo(7)'
Variable 'x' is not declared
[...]

[masak]

Bisecting this one was hard. Partly because Rakudo changed semantics recently (see cbc2ee2), and so I needed to float a stash down the bisect.

But the (tentative) conclusion of the three bisects I did was that this never worked. We never had a revision of 007 where the above did what we wanted. Which surprises me a little, actually. I was pretty sure we did.

[masak]

Weak evidence it never worked: t/features/quasi.t doesn't contain a test that assigns anything to a variable declared inside a quasi. It contains some my declarations in quasis (in order to test quasi @ Q::Statement), but it doesn't assign anything to them.

...and sure enough, it's the assignment that blows up, not the my declaration. Well, they're connected; you need to assign to something that isn't available outside for it to blow up.

        my x = 5;
        return quasi {
            my x = 42; # works
        }

        my x = 5;
        return quasi {
            my y = 42; # explodes
        }

[masak]

The parameter to the macro is not necessary either:

$ bin/007 -e='macro moo() { return quasi { my x = 1 } }; moo()'
Variable 'x' is not declared

Here's the rule for the my statement:

rule statement:my {
    my [<identifier> || <.panic("identifier")>]
    { declare(Q::Statement::My, $<identifier>.ast.name.value); }
    ['=' <EXPR>]?
}

Note the declare(...) call. The problem causing the present issue is that no such declare(...) call ever runs for the macro-expanded code. And it should.

[masak]

I'm staring at the implications of this issue and slowly coming to the conclusion that Q::Expr::StatementListAdapter needs to be a block.

Why? Because the my x in the one-liner above needs to leave an imprint on something. A my binds to its innermost surrounding block. This needs to be true even for expanded code. The only sensible place to have that block is exactly where the Q::Expr::StatementListAdapter is located.

Need to test this in a branch. First introducing the block into Q::E::SLA, then wiring up so that the my statement declares into that block.

Might also be a good idea to make sure that things related to static lexpads (such as const declarations) work out correctly, too.

[masak]

Making some headway. I just pushed a branch some-quasi-mishap-debugging which has one static-lexpad fix and some debug statements. On the simple breaking example above, I get this:

$ bin/007 -e='macro moo() { return quasi { my x = 1 } }; moo()'
Looking up moo
  looking in pad with 1 names: moo
Looking up moo
  looking in pad with 1 names: moo
Looking up moo
  looking in pad with 1 names: moo
Looking up --RETURN-TO--
  looking in pad with 2 names: --RETURN-TO--, moo
copying name from static lexpad: moo, (my \Val::Macro_8199
  moo is now declared. local pad has 1 entries
A
copying name from static lexpad: x, Val::NoneType.new
  x is now declared. local pad has 1 entries
B
Looking up x
  looking in pad with 2 names: --RETURN-TO--, moo
  ...not found here
  looking in pad with 1 names: moo
  ...not found here
  looking in pad with 120 names: Q::Expr::BlockAdapte
  ...not found here
Variable 'x' is not declared
  in method find-pad at /home/masak/ours/007/lib/_007/Runtime.pm (_007::Runtime) line 85
  in method put-var at /home/masak/ours/007/lib/_007/Runtime.pm (_007::Runtime) line 109
  in method put-value at /home/masak/ours/007/lib/_007/Q.pm (_007::Q) line 172
  in method run at /home/masak/ours/007/lib/_007/Q.pm (_007::Q) line 820
  in method run at /home/masak/ours/007/lib/_007/Q.pm (_007::Q) line 1068
  in method eval at /home/masak/ours/007/lib/_007/Q.pm (_007::Q) line 1097
  in method run at /home/masak/ours/007/lib/_007/Q.pm (_007::Q) line 846
  in method run at /home/masak/ours/007/lib/_007/Q.pm (_007::Q) line 1068
  in method run at /home/masak/ours/007/lib/_007/Q.pm (_007::Q) line 900
  in method run at /home/masak/ours/007/lib/_007/Runtime.pm (_007::Runtime) line 26
  in block  at bin/007 line 16
  in sub run_007 at bin/007 line 32
  in sub MAIN at bin/007 line 39
  in block <unit> at bin/007 line 39

I feel like a less tired version of me might be able to look at that and figure out where some scoping or other goes wrong.

For now, I'll just note that x does get declared properly (in the code expanded from the macro), but later for whatever reason it isn't found.

[masak]

Whee. If I make this change:

diff --git a/lib/_007/Q.pm b/lib/_007/Q.pm
index 1ec7222..3c6d87e 100644
--- a/lib/_007/Q.pm
+++ b/lib/_007/Q.pm
@@ -726,7 +726,7 @@ class Q::Term::Quasi does Q::Term {
             return $thing
                 if $thing ~~ Val;
 
-            return $thing.new(:name($thing.name), :frame($runtime.current-frame))
+            return $thing.new(:name($thing.name))
                 if $thing ~~ Q::Identifier;
 
             if $thing ~~ Q::Unquote::Prefix {

Then the simple breaking example:

$ bin/007 -e='macro moo() { return quasi { my x = 1 } }; moo()'

doesn't die with the Variable 'x' not declared message any more. But it leads to test failures in these files, all related to hygiene:

• t/examples/format.t
• t/examples/name.t
• t/features/builtins/methods.t
• t/features/quasi.t

Also, the same change applied to master doesn't fix the problem, likely because something in the statement-block-adapter fixes has changed the landscape somewhat.

This is complex, and I'm no less tired today.

[masak]

All of the test failures boil down to the same problem, golfed here to remove inessentials:

$ bin/007 -e='macro moo() { my x = 7; return quasi { say(x) } }; moo()'
Variable 'x' is not declared
[...]

(Actually, that golf is quite literally the failing test in quasi.t, modulo the fact that I still named macros foo back in 2015, not moo.)

[masak]

Let's analyze this.

-            return $thing.new(:name($thing.name), :frame($runtime.current-frame))
+            return $thing.new(:name($thing.name))
                 if $thing ~~ Q::Identifier;

In order to fix the present issue, it seems we need to make this change. Making the change causes four test failures related to hygienically finding variables from the quasi's environment.

I think that the :frame($runtime.current-frame) thing is entirely wrong, and that what we really want to do is something quite different. Let's see if I can put this into words.

(Parenthetical remark: we still don't have a word yet for "the code produced by injecting into the mainline the Qtree returned from a macro (typically produced by a quasi)". Note that this code is not the quasi code itself. It can't be, since a quasi is code-with-holes-in-it. Interpolating the quasi to produce a concrete code block means deep-cloning the quasi completely, filling the holes. Since we still don't have a good word for this, let's for the duration of this issue comment coin a silly one: "conifaq", or "concrete code interpolated from a quasi". Korzybski would be proud.)

Stating our goals:

1. A conifaq should be able to declare its own variables. (Again, this issue is mostly about making that work properly.)
2. A conifaq's OUTER should be the originating quasi's OUTER.
3. Occasionally we want to unhygenically allow a variable to refer to the mainline. But that's always opt-in, since we enjoy hygiene.

Hygiene could be defined as the first two goals together.

It would seem to me that the :frame(...) fix we need to roll back has given us a sort of partial, buggy version of goal 2.

Here, let me show you what I mean:

$ bin/007 -e='macro moo() { my x = 1; return quasi { my x; say(x) } }; moo()'
1

$ bin/007 -e='macro moo() { my x = 1; return quasi { { my x = 7 }; say(x) } }; moo()'
7

The first one should print None (because the quasi's x has not been assigned to). The second one should be 1 (because the x in the inner block in the quasi is unrelated to the x from outside the quasi that we access in say(x)).

The pathology can be summarized as "attach all identifiers in the quasi (regardless of depth) to the scope outside the quasi (typically the macro's scope)".

This is wildly, hilariously wrong.

The :frame(...) fix was added in this commit:

commit 045588a89c0ec82109191fa0acb632b8350062a0
Author: Carl Masak <cmasak@gmail.com>
Date:   2015-12-08 22:55:37 +0100

    give Q::Identifier an (optional) frame
    
    This is the mechanism which (I hope) will carry us all the way
    with various shenanigans macros get up to. Wrapping everything
    in carefully crafted blocks doesn't seem to be enough.
    
    (See http://irclog.perlgeek.de/6macros/2015-11-18#i_11557980 for
    some more context.)
    
    As a result, Q::Expr::Block is now unnecessary, and is removed,
    simplifying the overall design.

I'd just like to point out that Q::Expr::Block is now resurrected, in the form of Q::Expr::BlockAdapter. Seems we've been walking in a circle for the past two years... but that's fine too as long as we come back to the original point having learned something.

[masak]

As to achieving goal 1 the right way: one thing I've come to understand at depth since 2015 can be summarized like this:

Conifaqs are synthetic.

(Hey, we'd better come up with a decent alternative to the silly term before it spreads.)

Let's unpack this.

• ASTs come in two flavors: natural and synthetic.
• Natural ASTs were generated by parsing source code and generating ASTs from that.
• Synthetic ASTs were created by other means; the Qnodes thus created were never source code.
  • One way to do that is to simply create a Qnode in code: new Q::Identifier { name: "foo" }. This can then be injected into the program.
  • Another way is to clone a Qnode. The original Qnode might well have been natural, but the clone isn't.
• Conifaqs are cloned from quasi blocks. It's right there in the silly name. And so they're synthetic.

Ok, but what does it matter? Mainly because of this: when we parse a program, we also do a lot of work to prepare an AST for being run later. Most importantly, blocks are entered, variables are declared, static lexpads are built. This needs to happen for synthetic blocks too; logically, the only time to do it is as they are injected into the mainline.

I posit that doing so would give us all of goal 1 without the :frame(...) fix. Goal 2 is simple; just mess with the conifaq's OUTER. Goal 3 might require messing with identifiers' frames, but that ought to be doable also.

[masak]

Oh, and hey: instead of "conifaq", I propose we call the thing an injection:

• It's a real world. It's in common everyday use.
• It has ties to real CS use too: I think macro terminology mostly talks about "expanding" a macro, but "injecting" it into mainline code doesn't sound too foreign either.
• It pinpoints and highlights the most important aspect of the injected code: by being injected, it's automatically synthetic, because it just made it into the program's AST. It didn't get there by parsing some text.

[masak]

Or injectile. It's more noun-y than "injection", and it's analogous with "projectile", something that is projected.

[masak]

Branch statement-block-adapter, which I just merged to master, seems to fix all the versions of the bug in this issue except for the first one containing the anon sub.

Still, that's rather encouraging.

[masak]

This one (the original bug report of this issue) now works:

$ bin/007 -e='macro moo(x) { return quasi { (func() { my y = {{{x}}} })() }; }; say(moo(42))'
42

The shorter/golfed cases also work (but don't output anything when they do):

$ bin/007 -e='macro moo(a) { return quasi { my x = {{{a}}} } }; moo(7)'
$ bin/007 -e='macro moo() { return quasi { my x = 1 } }; moo()'

Lexical scoping now works from the injectile to the macro:

$ bin/007 -e='macro moo() { my x = 7; return quasi { say(x) } }; moo()'
7

Including the two cases constructed to demonstrate the pathology — which now show the expected behavior:

$ bin/007 -e='macro moo() { my x = 1; return quasi { my x; say(x) } }; moo()'
None

$ bin/007 -e='macro moo() { my x = 1; return quasi { { my x = 7 }; say(x) } }; moo()'
1

I see nothing further that needs fixing in this issue. Closing.