Skip to content

Add Rationale.md#390

Merged
jfbastien merged 56 commits intomasterfrom
rationale
Oct 8, 2015
Merged

Add Rationale.md#390
jfbastien merged 56 commits intomasterfrom
rationale

Conversation

@jfbastien
Copy link
Member

Move some of AstSemantics.md to that document. I left things pretty much as-is for now, and will defer more contended discussions to subsequent (and more targetted) edits.

Move some of AstSemantics.md to that document. I left things pretty much as-is for now, and will defer more contended discussions to subsequent (and more targetted) edits.
@jfbastien jfbastien added this to the MVP milestone Oct 6, 2015
Rationale.md Outdated
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you link explicitly to AstSemantics.md#types?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@lukewagner
Copy link
Member

Moving the rationale out of AstSemantics.md into its own doc is great. There's a number of FAQ entries that could perhaps be all moved to Rationale.md, replacing them with a Question "Why is _ specified as _?" who's Answer is "For the rationale behind various technical decisions in AstSemantics.md, see Rationale.md".

Rationale.md Outdated
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are the above two paragraphs necessary? It's a rationale document; its own purpose should be self-evident. And, I don't fully agree with the characterization of the process given here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's useful :-)

Which part don't you agree with? I'm open to changing it, but I do think the text makes stuff easier to understand.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you think people reading this don't know what a rationale is, how about just "This document explains some of the decisions made in the WebAssembly design."

There has been a lot of Science going on around here, and this introduction feels like we're saying we've regularly sourced major parts of the spec from thin air.

And finally, saying "desisions were made with a solid rationale" feels like something we wouldn't say this unless there's doubt as to whether it is true.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed for now, will move it to its own PR.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the intro, it says that the rationale for limited local nondeterminism is in its own document, but then there's a section here too.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes, I just updated that first paragraph, and added a link from Nondeterminism.md to here. The goal is to expand that section of Rationale.md later, since it may be a bit more contended.

Rationale.md Outdated
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like a typo on "and"

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Rationale.md Outdated
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about something like "The MVP's feature set is focused primarily on C/C++, but other source languages have different semantics in some cases. As we pursue support for more languages, we'll likely need to add more features, and generalize existing ones." ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure that's quite true: if it were the case we'd avoid specifying e.g. shift and divide the way we do! I'm really hoping that we start of with something that seems good, and before MVP we adjust what we have in preparation for other languages.

Rationale.md Outdated
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WebAssembly already does take extensive advantage of IEEE-754. What are you implying that it's missing here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm saying all relevant HW does IEEE-754, so there's no point to spec otherwise (even if forth would like us to).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already take advantage of IEEE 754 (with only a few specific details otherwise, but that's a different topic), so this doesn't seem like something there "may be a need to revisit".

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that's what I'm saying?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm reading it and that's not what I'm getting?

there may be a need to revisit some of the decisions:

When all relevant hardware implement features the same way then there's no need to get creative and add nondeterminism to WebAssembly when realistically there's only one mapping from WebAssenbly expression to ISA-specific operations.

To me, this reads like you're saying that WebAssembly currently has places where we got "creative" and "there may be a need to revisit" this "decision".

One such example is floating-point, where at a high-level most basic instructions follow IEEE-754 semantics.

And this suggests that we made up our own floating point semantics and that in the future "there may be a need to revisit" this "decision" and adopt IEEE 754.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I see what you mean. That's not what I wanted to get across, I hope this new update is better.

@jfbastien
Copy link
Member Author

Alright this should be ready to land as I've moved all the contended stuff out and will file separate PRs for them. Good to go? @sunfishcode / @lukewagner

Rationale.md Outdated
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to reword b/c there are no "following primitives".

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@lukewagner
Copy link
Member

lgtm with two tiny nits above

@titzer
Copy link

titzer commented Oct 8, 2015

lgtm; we can wordsmith individual sections in follow up PRs.

@sunfishcode
Copy link
Member

lgtm for now; we can continue to iterate in follow-up PRs.

jfbastien added a commit that referenced this pull request Oct 8, 2015
@jfbastien jfbastien merged commit 585f95e into master Oct 8, 2015
@jfbastien jfbastien deleted the rationale branch October 8, 2015 20:35
This was referenced Oct 8, 2015
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.

5 participants