shell.js environment cleanup

[kripken]

• Use console.log, it is present in all modern environments (use LEGACY_VM_SUPPORT to use older methods for really old shells).
• No need to check if a property exists on Module already, we handle that in a general way, user-provided things will not be overridden (see moduleOverrides).

[rongjiecomputer]

While you are at it, can you add a var here?

[kripken]

There's already a var key; in the top scope, before the first loop on key in moduleOverrides,

https://github.com/kripken/emscripten/blob/2c5e89c662621e68d2cf4f3a7e237ad3a1c6b358/src/shell.js#L34

[juj]

LEGACY_VM_SUPPORT is a bit vague - what is considered a legacy VM, and what does it enable? Can we directly name the feature something like SUPPORT_OLD_SPIDERMONKEY_CONSOLE_PRINT or similar? (I presume this is for old SpiderMonkey that did not have console.log but had print?)

[kripken]

Not just spidermonkey, d8 also had print and printErr early on, and I think webkit had print but not printErr (but less sure about that, I tested less on it).

Honestly I think we can just drop this section, but I figured since LEGACY_VM_SUPPORT already exists, why not let it cover this as well? Basically anyone using a non up-to-date VM can use that and get backwards compatibility, but most people don't need to think about it.

[juj]

Oh now I see indeed we have a LEGACY_VM_SUPPORT feature already, missed that it already existed. I feel that's not the kind of way we'd want to feature test for compatibility features, since it's either "all legacy" or "no legacy", without documenting what the actual legacy feature(s) are and which versions of software need which legacy features. I think we'd rather remove LEGACY_VM_SUPPORT and replace it with feature-specific or client-specific #ifdefs (whichever is easier?) Something like that was the idea with #5554. Perhaps we could have #ifdef SUPPORT_JS_VM_CONSOLE_PRINT and #ifdef POLYFILL_PRIMITIVE_MATH_OPS or similar, so that we would not put multiple features behind the same flag, but have one define per feature? Then the name will illustrate what the actual features are.

For related reference:

• SpiderMonkey documented console.log for SpiderMonkey 53, (which was released on 2017-04-19): https://developer.mozilla.org/en-US/docs/Mozilla/Projects/SpiderMonkey/Shell_global_objects$compare?locale=en-US&to=1151230&from=1004343

• It looks like d8 does not yet have console.log, from https://bugs.chromium.org/p/v8/issues/detail?id=4986 . (or could be that bug report is out of date?)

• From https://stackoverflow.com/questions/34914397/why-doesnt-console-log-work-in-the-jsc-environment-but-it-works-in-safaris-deb/34914888 and https://stackoverflow.com/questions/19649932/javascriptcore-console-log?noredirect=1&lq=1, it looks like jsc did not have console.log at least in May 2017.

[kripken]

On the one hand, I agree that for commonly useful features we should have specific flags. But I see legacy support as different, it's rarely used, and it's code we are very close to just removing entirely.

For those reasons, a single flag for all legacy stuff seems ok to me. But if I'm in the minority here I don't feel strongly.

[rongjiecomputer]

It looks like d8 does not yet have console.log, from https://bugs.chromium.org/p/v8/issues/detail?id=4986 . (or could be that bug report is out of date?)

The bug is out-dated. d8 does have console.log, along with other console methods. See https://cs.chromium.org/chromium/src/v8/src/d8-console.cc?l=59

[dschuff]

Does jsc have console.log?

[kripken]

@jfbastien, does the jsc shell have console.log and console.warn?

[dschuff]

In my build it looks like the answer is no.

[jfbastien]

We have print and printErr.

[kripken]

Ok, thanks, I restored print/printErr support for jsc. Also added a few more minor cleanups in that file.

[kripken]

Any concerns here?