Skip to content

Conversation

@rossberg
Copy link
Member

@rossberg rossberg commented Mar 8, 2016

(And while here, do a stylistic tweak to use : for type annotations :) )

@lukewagner
Copy link
Member

+1 for syntactic arity immediate. The opcodes-with-fixed-immediates extension (which I'll make a PR for once we finish convergence on the current binary format) should hopefully mitigate the size impact.

Conflicts:
	BinaryEncoding.md
@rossberg
Copy link
Member Author

Ping. Now that GDC is over, this should be ready to land.

@sunfishcode
Copy link
Member

lgtm

@binji
Copy link
Member

binji commented Mar 28, 2016

Shouldn't this mention that arity mismatch is an error?

@lukewagner
Copy link
Member

lgtm too

@ghost
Copy link

ghost commented Mar 28, 2016

I support the parsing not depending on the context, but still see potential problems with the return and break operators having an 'arity'.

I believe that the appropriate solution would be for the return and break operators to take a single expression argument, and that in the cast that they return zero values that this be a nop. When multiple value supported is added it would add a tuple operator that could be used here as the single expression or in the fall-through expression to return multiple values.

While the 'arity' of the return and break operators could be interpreted a zero meaning a single nop argument and one meaning a single expression for which all values are returned, which I would accepts as a concession, the current semantic disallow a nop as the single expression which cast serious debts on this interpretation. Further there have been suggestions of this arity supporting multiple values in future, casing more doubt on this.

I ask that if this lands that the arity of the return and break operators be revisited before the MVP, but if landing this PR is to be interpreted as resolving the arity of the return and break operators for the MVP then please let me know so that I can continue to make the case here?

@MikeHolman
Copy link
Member

I can sympathize with the idea that parsing should not rely on its context, and I can kind of see how tooling might want to read ASTs without having to parse the whole module, but if I'm doing the work to parse the whole AST for function bodies... it is not much more work to read the signature and declaration sections.

I debate the point @titzer made that "redundancy can catch encoding errors in mismatches". Redundancy can cause bugs where someone is validating one number and then using another, etc. It is much safer to have a value stored in a single place rather than multiple places. I place higher precedence on the elimination of these bugs (which can often be security vulnerabilities) than making this more context-free.

@ghost
Copy link

ghost commented Mar 30, 2016

@titzer If you believe that one byte per function for an end marker is significant then you might be interested to know that AngryBots has 195682 function calls and one extra byte each is about five times as much as the function end markers and 1.6% of the file size. This seems significant although I also see some benefit to being context-free. As noted above the operator table might help here with call0 call1 etc.

@ghost
Copy link

ghost commented Apr 19, 2016

From the perspective of an expression-less encoding, the return statement is the only way to return values from a function, there is no option to use a tuple/values expression to create expression values to return, and from this perspective an arity on the return is necessary.

Also from this perspective the call operator might need an arity for both the number of result local variables and the number of argument local variables, but only if not using the signature for this information.

In an expression-less encoding blocks do not return values, so breaks need no arity.

@kripken
Copy link
Member

kripken commented Apr 25, 2016

I did some measurements, this adds 1.3% to BB's binary size, or 0.4% after gzip. So noticeable, but quite small. @lukewagner is also optimistic that that would go away with an opcode table. I think he's probably right, and given the small downside here it seems not too worrying. But just noting these numbers here, it would be nice to measure later on after an opcode table as well, as technically speaking this information is redundant in the binary format.

@titzer
Copy link

titzer commented Apr 27, 2016

This PR needs to be rebased onto the binary_0xb branch.

@rossberg
Copy link
Member Author

@titzer, the rebase already landed as #672, just forgot to close this one.

@rossberg rossberg closed this Apr 27, 2016
@titzer
Copy link

titzer commented Apr 27, 2016

Thanks!

@jfbastien jfbastien deleted the arities branch April 27, 2016 16:22
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

Successfully merging this pull request may close these issues.

7 participants