Add LocalVariableTable for output to support debug#44
Add LocalVariableTable for output to support debug#44warmthdawn wants to merge 1 commit intoCraftTweaker:masterfrom
Conversation
kindlich
left a comment
kindlich
left a comment
There was a problem hiding this comment.
Some questions:
- This Repository is for ZenScript1, so the version that 1.7.10 up to 1.12 use, that is intended?
- IIRC, in ZenCode, we put this information as part of the MethodOutput object, and have
methodOutput.end()write automatically (so it can't be forgotten for some method types). Any reason to put it in the scope here?
Possible Edge cases
- Nested scopes that don't belong together (e.g. local variables inside a lambda shouldn't be added to the outer variable table)
- Variable Capturing in Lambdas
- Naming object0
thisfor instance methods in zenClasses vs. static methods where object0 is the first parameter - Names of Temp variables that some converters introduce (e.g. the List->Array caster that also converts elements)
- Variables Captured within Lambdas
Other things to keep in mind:
- Currently the
legacybuilds of CraftTweaker might go against a different Commit than the master commit of this repo: e714482. When testing, make sure to properly update the ZenScript repo for your CrT stuff.
https://github.com/CraftTweaker/CraftTweaker/tree/legacy
Also, Make sure you have an old enough version of gradle running for that project 😛
If you want to discuss more, I'd suggest you join our discord server
| public class LocalVariableTable { | ||
|
|
||
| private final IEnvironmentClass envClass; | ||
| private final LinkedList<List<LocalVariable>> scopeStack = new LinkedList<>(); |
There was a problem hiding this comment.
IF the outer list is only ever used as Stack, define it as such
private final Stack<List<...>> .. = new LinkedList<>();There was a problem hiding this comment.
Yes, the outer is only ever used as stack, however Stack is not a superclass of LinkedList. 😛
| public void beginScope() { | ||
| scopeStack.push(new ArrayList<>()); | ||
| } | ||
|
|
||
|
|
||
| public void endScope(Label end) { |
There was a problem hiding this comment.
How would this work for nested scopes that have nothing to do with each other?
E.g.
function myFunc() as int {
val someLambda as function()int = function() {
val x = 10;
return x;
}
return someLambda();
}There was a problem hiding this comment.
Well, the 'scope' I defined here is for cases of block expression, scopes like lambda creates a new stackframe thus has no impact on owner's local variable tables, even if it has a capture variable, it is provided separately without affecting the local variable.
actually this is just a trick to have access of the end label prop for local variable.
| if(firstLabel == null) { | ||
| firstLabel = label; | ||
| } | ||
| lastLabel = label; |
There was a problem hiding this comment.
Why these instead of setting firstLabel in start() and lastLabel in end()?
| if(firstLabel == null) { | ||
| firstLabel = label; | ||
| } | ||
| lastLabel = label; |
There was a problem hiding this comment.
Since this now became a bit more duplicated code you could delegate to label?
posision(pos) {
Label label = new Label
this.label(label);
visitor.visitLineNumber(...);
}There was a problem hiding this comment.
Yes, the first label would be initialized later when the first expression compiles.
| .map(ZenType::toASMType) | ||
| .map(Type::getDescriptor) | ||
| .toArray(String[]::new); | ||
| final String[] arguments = environmentMethod.getCapturedVariables().stream().map(SymbolCaptured::getEvaluated).peek(expression -> expression.compile(true, environment)).map(Expression::getType).map(ZenType::toASMType).map(Type::getDescriptor).toArray(String[]::new); |
There was a problem hiding this comment.
Why the reformat instead of keeping it chopped?
There was a problem hiding this comment.
Sorry, this is just a mistake because i forge to sync code style to ide at first and forge to rollback some changes. I'll settle that later
| .map(ZenType::toASMType) | ||
| .map(Type::getDescriptor) | ||
| .toArray(String[]::new); | ||
| final String[] arguments = environmentMethod.getCapturedVariables().stream().map(SymbolCaptured::getEvaluated).peek(expression -> expression.compile(true, environment)).map(Expression::getType).map(ZenType::toASMType).map(Type::getDescriptor).toArray(String[]::new); |
There was a problem hiding this comment.
Why the reformat instead of keeping it chopped down?
| return isStatic | ||
| ? members.get(name).instance(position, environment) | ||
| : members.get(name).instance(position, environment, value); | ||
| return isStatic ? members.get(name).instance(position, environment) : members.get(name).instance(position, environment, value); |
There was a problem hiding this comment.
The the reformat instead of keeping it chopped down?
| if (!create) { | ||
| return -1; | ||
| } |
There was a problem hiding this comment.
Default value vs. Exception/Error?
There was a problem hiding this comment.
o, right，error here would be better
|
As for why I didn't write the final output in end(), first of all, there are some methods (such as lambda-generated temporary methods) that don't need these. Second, the generation needs to get something related to Environment, and it seems too much for MethodOutput. this pr is intended to mc1.12, BTW, for zencode, since it wirtes the 'end' immediately next to start label, i wonder whether there are any problems. |
2 participants
As title， write local variable table to the compile output in order to support debug.
(The debug is provided by a vscode plugin I'm developing)
https://github.com/warmthdawn/zs-debug-adapter