Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

In compilation environment, use name + ordinal as the key, not just name, to accommodate variable copies caused by inlining #179

Closed
julianhyde opened this issue Sep 25, 2022 · 0 comments

Comments

@julianhyde
Copy link
Collaborator

In the compilation environment (net.hydromatic.morel.compile.Environment) the key for values was previously a String (the name of the variable) but is now a Core.NamedPat (the name plus an ordinal). This accommodates duplicate names, which can occur by certain kinds of inlining.

For example, if you inline

from e in (from e in emps yield {e.deptno, e.ename})
where e.deptno = 30

you get

from e in emps
  yield {e.deptno, e.name}
  yield {e_1 = e}
  where e_1.deptno = 30

(where e_1 is a variable whose name is "e" and ordinal is 1 and e is a variable whose name is "e" and ordinal is 0). The variables shadow each other before inlining but they need to coexist after inlining.

As part of this change we add class RefChecker, to check that variables referenced are in scope. If, in the above, we changed where e_1.deptno = 30 to where e.deptno = 30, RefChecker would notice that the output bindings of the yield {e_1 = e} step contained e_1 and not e, and. therefore where e.deptno = 30 is invalid.

And we fix EnvShuttle and EnvVisitor for problems found by RefChecker.

julianhyde added a commit to julianhyde/morel that referenced this issue Sep 25, 2022
…, not just name, to accommodate variable copies caused by inlining

Add class RefChecker to detect bad references in FromBuilder.

In EnvVisitor, improve the environment passed to Group;
in EnvVisitor and EnvShuttle, RecValDecl adds to the
environment in which the expression body is processed.

Generate fewer new variables when copying a Yield step.

When copying a Yield step like '{a = exp1, b = exp2}', inherit
bindings from the previous version of the step so that the
'a' and 'b' varibles have the same ordinal, and will match
the variables needed by the next step.

We can remove a trivial Yield, but 'yield {e=e}' isn't
trivial if the output IdPat has a different ordinal than the
input IdPat.

Fixes hydromatic#179
@julianhyde julianhyde reopened this Oct 2, 2022
julianhyde added a commit to julianhyde/morel that referenced this issue Oct 3, 2022
…, not just name, to accommodate variable copies caused by inlining

Add class RefChecker to detect bad references in FromBuilder.

In EnvVisitor, improve the environment passed to Group;
in EnvVisitor and EnvShuttle, RecValDecl adds to the
environment in which the expression body is processed.

Generate fewer new variables when copying a Yield step.

When copying a Yield step like '{a = exp1, b = exp2}', inherit
bindings from the previous version of the step so that the
'a' and 'b' varibles have the same ordinal, and will match
the variables needed by the next step.

We can remove a trivial Yield, but 'yield {e=e}' isn't
trivial if the output IdPat has a different ordinal than the
input IdPat.

Fixes hydromatic#179
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant