Add stdio.write import for writing a sequence of bytes to stdout #121

kg · 2015-10-09T18:28:03Z

No description provided.

sunfishcode · 2015-10-09T18:36:05Z

ml-proto/test/stdio_write.wast

+(module 
+  (import $write "stdio" "write" (param i32 i32))
+
+  (memory 4096 4096 (segment 0 "hello, world!\0D\0A\00"))


In C/C++ code compiled to wasm, strings don't contain the "\0D", because stdout is in "text" mode, which ordinarily means that the C library translates newline into carriage-return-line-feed on Windows. However in wasm's case, the C library won't know that it's running on Windows, so it won't know that it has to do this. I think it's best if stdio.write handles this itself so that wasm programs don't have to worry about it.

That would make it unusable for binary data.

The alternative is to be unusable for text strings in C/C++. How about a separate function for binary data?

write is a base primitive for writing whatever to stdout. if you want to print text, your stdlib would do any transforms like \n -> \r\n on windows.

How should the stdlib detect that is is on Windows?

What I'm actually concerned about here is that there's wasm code with a \r\n in it, which I find very concerning. I assumed you put it there because of Windows; if not, then can we just remove the \r?

IIUC this is meant to test that \r is unchanged. I strongly believe that fwrite shouldn't change the bits it's given, so the test should exist in some form. Are you suggesting the form be changed?

Would you mind making a separate test for binary output then? I saw this test as the "hello world" test that would be used as an example of what wasm code looks like. I don't want to risk anyone thinking that wasm code needs a \r to print hello world.

As you say, a separate test for binary output could test \0 and other interesting characters.

"write" for binary data, "print" for text?

On Fri, Oct 9, 2015 at 12:34 PM, Dan Gohman notifications@github.com
wrote:

In ml-proto/test/stdio_write.wast
#121 (comment):

@@ -0,0 +1,14 @@
+(module

(import $write "stdio" "write" (param i32 i32))

(memory 4096 4096 (segment 0 "hello, world!\0D\0A\00"))

Would you mind making a separate test for binary output then? I saw this
test as the "hello world" test that would be used as an example of what
wasm code looks like. I don't want to risk anyone thinking that wasm code
needs a \r to print hello world.

As you say, a separate test for binary output could test \0 and other
interesting characters.

—
Reply to this email directly or view it on GitHub
https://github.com/WebAssembly/spec/pull/121/files#r41669560.

I think separate test files is fine.

kg · 2015-10-10T18:40:47Z

Welp. Looks like windows ocaml is doing line normalization. I probably need to find another way to write raw bytes to stdout.

======================================================================
FAIL: test/stdio_write.wast (__main__.RunTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "./runtests.py", line 37, in <lambda>
    return lambda self : self._runTestFile(*rec)
  File "./runtests.py", line 34, in _runTestFile
    self.assertEqual(expectedText, actualText)
AssertionError: '\x89PNG\r\n\x1a\n\x00' != '\x89PNG\r\r\n\x1a\r\n\x00'

pjuftring · 2015-10-10T21:19:43Z

I added the line

set_binary_mode_out stdout true;

And it seems to work, but I don't know if it does on linux.

kg · 2015-10-12T18:27:12Z

Switched to print_bytes, that might disable newline conversion. Or it might not, since Bytes is initialized from char (?????)

rossberg · 2015-10-13T09:24:05Z

I don't think print_bytes disables conversion (have you tried it?).

It's just like in the C or C++ stdlibs: newline conversion is a property of the stream, not the source. And stdout is a text stream. If you want to disable newline conversion on Windows, you need to go to binary mode. As @pjuftring pointed out, you can use set_binary_mode_out to change the mode after the fact. (Just make sure to switch it back before the function returns, so that you don't break other output to stdout.)

kg · 2015-10-13T14:34:09Z

I don't think print_bytes disables conversion (have you tried it?).

Nope, I don't have access to Windows at work :-) I'll come up with a fix on my Windows machine.
Good point about the pattern match, didn't realize you could use them on lists.

kg · 2015-10-13T15:37:34Z

Yeah, print_bytes doesn't disable conversion. Weird :( I'll just do set_binary_mode_out. One caveat is that you can't query the current binary mode state, so there's no way to restore it after setting it - as a result this function will trample over the output mode. I don't like that, but I can't see an alternative.

Your suggested pattern-match doesn't seem like it type-checks, at least as written. I'm not actually sure why, as it looks correct to me.

Error: This expression has type
         Eval.instance ->
         ((int32, 'a, 'b, 'c) Values.op * (int32, 'd, 'e, 'f) Values.op) list ->
         'g option
       but an expression was expected of type
         Eval.instance -> Values.value list -> 'h option
       Type (int32, 'a, 'b, 'c) Values.op * (int32, 'd, 'e, 'f) Values.op
       is not compatible with type
         Values.value = (I32.t, I64.t, F32.t, F64.t) Values.op

Should I just update the PR without using the pattern-match, or do you not want to merge without it? The pattern-match does seem like the right way to do this.

rossberg · 2015-10-13T15:58:45Z

Ah, sorry, I keep forgetting that OCaml separates list items by semicolons. That should fix it.

…rmalized. Iterate using Int64 (since the offset and count are Int64)

kg · 2015-10-13T18:04:55Z

OK, I've revised it so that it works correctly on Windows now.

Had some confusion over the fact that to disable newline normalization, you do set_binary_mode_out stdout false - the meaning of the arg is reversed.

rossberg · 2015-10-14T20:00:16Z

lgtm

sunfishcode · 2015-10-14T21:30:10Z

ml-proto/host/builtins.ml

@@ -1,9 +1,50 @@
 open Source
+open Eval


From a readthrough, the only use of the Eval module in this file is the Eval.memory_for_module. Can this open be removed?

rossberg · 2016-02-25T01:09:39Z

Closing, there doesn't seem to be a strong need

@rossberg

…bly#121) Included in this PR: - Detailed core formal spec additions aiming to fully implement: + the "informal formal" core spec laid out by Andreas @rossberg here: WebAssembly/exception-handling#87 (comment) and + the core spec outlined in the [proposal overview](https://github.com/WebAssembly/exception-handling/blob/master/proposals/Exceptions.md), while removing the mention of "events" everywhere but in the binary format , + prose for the above in: syntax, validation, execution, binary format, text format, and the appendix, as well as an example for throw contexts. - Travis-ci build status icon to `README.md`. The contents of this PR get built and deployed on my fork's github page successfully, using [the howto instructions](https://github.com/WebAssembly/proposals/blob/master/howto.md#setting-up-travis-and-github-page). Not included in this PR: - Neither the new values representing unresolved throw contexts, nor the additional rules for the execution contexts, which we discussed in WebAssembly#87, are added here. I plan to add these separately. - No JS API/Web API changes. Side comments: - In [document/core/text/types.rst](https://ioannad.github.io/exception-handling/core/text/types.html#text-refedtype): I'm not sure whether `refedtype ::= ... | event ⇒ exnref` should have `event` there or something else (perhaps `exception`?) However, since currently the only events are exceptions, this is probably not really an issue. - The "informal formal spec" defines the administrative instruction which represents an exception package as `exnref`, i.e., the same name as the type. I called this administrative instruction `ref.exn` instead for two reasons; to match the style of `ref.null` and `ref.extern`, and to avoid ambiguity in the discussions. - I removed multi-value mentions from `README.md` because multi-value is now part of the main spec. For the same reason I didn't add "multi-value" to the list of included proposals at the top of the core spec landing page.

Fix expected result type in relaxed dot product tests

Add stdio.write import for writing a sequence of bytes to stdout

6df9516

sunfishcode reviewed Oct 9, 2015
View reviewed changes

kg added 2 commits October 9, 2015 13:23

Change stdio_write

f41e4b6

Update expected output

f59782a

Address style nits. Write output in binary mode so newlines aren't no…

6ff3c7e

…rmalized. Iterate using Int64 (since the offset and count are Int64)

kg force-pushed the stdio_write branch from d54d683 to 6ff3c7e Compare October 13, 2015 18:03

sunfishcode reviewed Oct 14, 2015
View reviewed changes

rossberg closed this Feb 25, 2016

Connicpu pushed a commit to Connicpu/wasm-spec that referenced this pull request May 11, 2020

Use local.get instead of get_local (WebAssembly#121)

b1dce23

rossberg pushed a commit that referenced this pull request Sep 4, 2024

Merge pull request #121 from ngzhian/fix-relaxed-dot-test

e7bb04d

Fix expected result type in relaxed dot product tests

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add stdio.write import for writing a sequence of bytes to stdout #121

Add stdio.write import for writing a sequence of bytes to stdout #121

kg commented Oct 9, 2015

sunfishcode Oct 9, 2015

kg Oct 9, 2015

sunfishcode Oct 9, 2015

kg Oct 9, 2015

sunfishcode Oct 9, 2015

sunfishcode Oct 9, 2015

jfbastien Oct 9, 2015

sunfishcode Oct 9, 2015

titzer Oct 9, 2015

jfbastien Oct 9, 2015

kg commented Oct 10, 2015

pjuftring commented Oct 10, 2015

kg commented Oct 12, 2015

rossberg commented Oct 13, 2015

kg commented Oct 13, 2015

kg commented Oct 13, 2015

rossberg commented Oct 13, 2015

kg commented Oct 13, 2015

rossberg commented Oct 14, 2015

sunfishcode Oct 14, 2015

rossberg commented Feb 25, 2016

Add stdio.write import for writing a sequence of bytes to stdout #121

Add stdio.write import for writing a sequence of bytes to stdout #121

Conversation

kg commented Oct 9, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kg commented Oct 10, 2015

pjuftring commented Oct 10, 2015

kg commented Oct 12, 2015

rossberg commented Oct 13, 2015

kg commented Oct 13, 2015

kg commented Oct 13, 2015

rossberg commented Oct 13, 2015

kg commented Oct 13, 2015

rossberg commented Oct 14, 2015

Choose a reason for hiding this comment

rossberg commented Feb 25, 2016