Restrict validation output to just validation errors in the API

[dcodeIO]

This restricts validation output to validation errors in the API, leaving additional and possibly very longish printing of the entire module up to the respective tool.

[kripken]

Hmm, I find this quite useful myself as a developer :) How else would you print out the erroring module? If you add --print after the pass that breaks it, it won't reach the print?

But if this is annoying it could be optional. Perhaps one of the current debug env vars (BINARYEN_PASS_DEBUG, BINARYEN_PRINT_FULL) could cover it, or we could add another.

[dcodeIO]

Imho the respective tool (using the API) should provide printing of the entire module as an option, not the API itself where one function should do one thing. For instance, one could simply do

if (!BinaryenModuleValidate(the_module))
  BinaryenModulePrint(the_module);

leading to the exact same output as before, while there is no way to print just the validation messages (which I am interested in) as it is atm. Just removing this line and leaving it up to the user to print the module contents where necessary also avoids adding more options (do env vars even work in JS?).

[kripken]

I see, I was thinking more form the point of view of a commandline tool. Yeah, I agree we should do it the way you propose, by making printing the module optional, and we should make it print the module in the commandline tools like wasm-opt, so the user of those tools doesn't see a change from this PR (while people making their own tools using the C API can choose to print the module or not).

[kripken]

(About env vars and js, emscripten does support getenv, but it isn't hooked up to the actual system env. But in node.js someone could in theory do that.)

[dcodeIO]

This commit should add explicit printing of the module to all places where it has been printed before but then wasn't due to the change.

[dcodeIO]

is this correct?

[kripken]

Hmm, this does change the semantics, before it would not even check if assertions are off. But I see no downside to making this change, probably better actually.

[kripken]

Overall this looks like the right direction, but the CI errors show that we are printing the module when testing spec/func_ptrs.wast, which suggests it doesn't validate? But that can't be since the test passes. I'd run it locally to see more (maybe there is stderr output that travis is hiding).

[dcodeIO]

I am a bit puzzled what's going on there.

Unfortunately, I cannot run the waterfall tests on Windows because there are no such binaries (downloading waterfall 13645: https://storage.googleapis.com/wasm-llvm/builds/windows/13645/wasm-binaries-13645.tbz2 ... HTTP Error 404: Not Found).

[dcodeIO]

Just checked: Local output is the same as shown on Travis.

[dcodeIO]

Seems to produce the expected output if I do this, but I must admit that I do not understand why validate didn't print the module at this occasion before. Only accountable candidate I observed there is info.quiet being true, but I can't spot any flags being provided.

[kripken]

Yeah, that's odd - it seems like it should have been printed before too. Anyhow, your fix is the right one, in wasm-shell we actually don't want to print it, it has tests that are supposed to fail, without printing.