Skip to content

Convert operator and type names to lowercase with underscores. #122

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

Merged
merged 1 commit into from
Jun 15, 2015

Conversation

sunfishcode
Copy link
Member

Yet another attempt to resolve #83.

@sunfishcode sunfishcode mentioned this pull request Jun 8, 2015
@lukewagner
Copy link
Member

Also me. On pure personal preference, lower_with_underscores is my favorite naming convention. But it's also good by avoiding the uncanny valley of being 1 capitalization away from two popular naming schemes.

@titzer
Copy link

titzer commented Jun 9, 2015

Runs faster too, because C.

On Tue, Jun 9, 2015 at 4:55 PM, Luke Wagner [email protected]
wrote:

Also me. On pure preferences, lower_with_underscores is my favorite naming
convention. But it's also good by avoiding the uncanny valley of being 1
capitalization away from two popular naming schemes.


Reply to this email directly or view it on GitHub
WebAssembly/spec#122 (comment).

@lukewagner
Copy link
Member

Another reason: there is necessarily going to be a mapping from WebIDL signatures into wasm signatures and this mapping will (I hope!) take Uint32Array not to some wasm view type, but to two parameters uint32* base, uint32 length (thereby avoiding temporary garbage). Thus, Uint32Array and the rest of the view types are necessarily separate kinds of things from anything in wasm.

@kripken
Copy link
Member

kripken commented Jun 9, 2015

Typed arrays will still be passable to wasm, though, as mentioned here? I don't think we can avoid Uint32Arrays etc. being a name that comes up in wasm.

Overall, this seems strictly worse than to follow existing web conventions, but if I am the only person that thinks that way, this pull seems to me like the least bad of all alternatives.

@lukewagner
Copy link
Member

@kripken Yes, that would be a separate way for wasm to operate on a real JS Uint32Array.

@sunfishcode
Copy link
Member Author

Ok, I closed #83 and #93 so that we can focus on this one approach. Do people like this? Dislike it?

Some of the names here, in particular load_heap_with_offset, int32_from_float32_bits, but possibly others too, may want further renaming, but the main goal here is to get consensus on the overall style first.

@jfbastien
Copy link
Member

I like this.

@MikeHolman
Copy link
Member

I agree with @kripken that I would prefer we follow existing web conventions (e.g. by using Uint32 style), but on the other hand I don't think it's a big deal.

@jfbastien
Copy link
Member

Is there an outcome on this?

@kripken
Copy link
Member

kripken commented Jun 11, 2015

@pizlonator is the only one I think I remember participating in the other discussions on this, that has not weighed in here with an ok. @pizlonator, thoughts?

@jfbastien jfbastien added this to the Public Announcement milestone Jun 12, 2015
@sunfishcode
Copy link
Member Author

Now updated to use dots instead of underscores when it looks natural to do so, so operators now look like int32.add and float32.mul, since reactions to this among people I've shown it to so far were all positive.

@jfbastien
Copy link
Member

As discussed on IRC: merging now so we can move forward and merge dependent PRs (#135, further refactoring); we can revisit naming later.

jfbastien added a commit that referenced this pull request Jun 15, 2015
Convert operator and type names to lowercase with underscores.
@jfbastien jfbastien merged commit 204b7b8 into master Jun 15, 2015
@jfbastien jfbastien deleted the lowercase-names branch June 15, 2015 14: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.

6 participants