Skip to content

[NFC] Some comments in settings.js #17410

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.

Sign up for GitHub

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 2 commits into from
Jul 11, 2022
Merged

[NFC] Some comments in settings.js #17410

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9cc9963
comment [ci skip]
kripken Jul 11, 2022
5f1b295
comment [ci skip]
kripken Jul 11, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 8 additions & 7 deletions src/settings.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,13 +49,10 @@

// Tuning

// Whether we should add runtime assertions, for example to
// check that each allocation to the stack does not
// exceed its size, whether all allocations (stack and static) are
// of positive size, etc., whether we should throw if we encounter a bad __label__, i.e.,
// if code flow runs into a fault
// Whether we should add runtime assertions. This affects both JS and how
// system libraries are built.
// ASSERTIONS == 2 gives even more runtime checks, that may be very slow. That
// includes internal dlmalloc assertions.
// includes internal dlmalloc assertions, for example.
// [link]
var ASSERTIONS = 1;

Expand Down Expand Up @@ -1732,6 +1729,7 @@ var AUTO_NATIVE_LIBRARIES = true;
// are desired to work. Pass -sMIN_FIREFOX_VERSION=majorVersion to drop support
// for Firefox versions older than < majorVersion.
// Firefox ESR 60.5 (Firefox 65) was released on 2019-01-29.
// MAX_INT (0x7FFFFFFF, or -1) specifies that target is not supported.
// [link]
var MIN_FIREFOX_VERSION = 65;

Expand All @@ -1742,26 +1740,29 @@ var MIN_FIREFOX_VERSION = 65;
// NOTE: Emscripten is unable to produce code that would work in iOS 9.3.5 and
// older, i.e. iPhone 4s, iPad 2, iPad 3, iPad Mini 1, Pod Touch 5 and older,
// see https://github.com/emscripten-core/emscripten/pull/7191.
// MAX_INT (0x7FFFFFFF, or -1) specifies that target is not supported.
// [link]
var MIN_SAFARI_VERSION = 120000;

// Specifies the oldest version of Internet Explorer to target. E.g. pass -s
// MIN_IE_VERSION = 11 to drop support for IE 10 and older.
// Internet Explorer is at end of life and does not support WebAssembly.
// MAX_INT (0x7FFFFFFF) specifies that target is not supported.
// MAX_INT (0x7FFFFFFF, or -1) specifies that target is not supported.
// [link]
var MIN_IE_VERSION = 0x7FFFFFFF;

// Specifies the oldest version of Edge (EdgeHTML, the non-Chromium based
// flavor) to target. E.g. pass -sMIN_EDGE_VERSION=40 to drop support for
// EdgeHTML 39 and older.
// Edge 44.17763 was released on November 13, 2018
// MAX_INT (0x7FFFFFFF, or -1) specifies that target is not supported.
// [link]
var MIN_EDGE_VERSION = 44;

// Specifies the oldest version of Chrome. E.g. pass -sMIN_CHROME_VERSION=58 to
// drop support for Chrome 57 and older.
// Chrome 75.0.3770 was released on 2019-06-04
// MAX_INT (0x7FFFFFFF, or -1) specifies that target is not supported.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does -1 really work? If so maybe just stick to that and drop the 0x7FFFFFFF magic?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, it does work. We have code to convert them in emcc.py.

I agree it makes sense to simplify this, and -1 is probably better. I can look into that as a followup.

Copy link
Member Author

Choose a reason for hiding this comment

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

Or I guess this is so small I could wait and do it 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.

Oh, reading the emcc.py code, it is necessary to use 0x7fffffff, as it's a large positive number, which the code depends on. So we just have some parsing support for -1 that we turn it into 0x7fffffff, but use the latter internally. Not sure that makes sense to change.

// [link]
var MIN_CHROME_VERSION = 75;

Expand Down