Closed Bug 1577373 Opened 5 years ago Closed 5 years ago

Split up builtin/Stream.* (similar to how builtin/intl was filled)

Categories

(Core :: JavaScript: Standard Library, task, P1)

task

Tracking

()

RESOLVED FIXED
mozilla70
Tracking Status
firefox70 --- fixed

People

(Reporter: Waldo, Assigned: Waldo)

Details

Attachments

(16 files)

47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review

builtin/Stream.* is something like 5500 lines. If you want to edit it while referring to separate portions of it, the best you can do is multiple editor windows (and hope your editor supports loading some of them readonly or something). If you want to find something in it, it's at least organized by spec section...but still, it's wicked huge.

We should create a new builtin/streams directory and populate it with a bunch of different files containing all the streams code, split up by spec section or so. The code won't be in literal spec order, but that seems less valuable to me than having things that can be compared easily and kept in mind more easily and where you can't completely lose yourself scrolling through. (Particularly if streams code grows a whole bunch of support for writable streams, piping, readable byte streams, &c. in the future.)

Similarly to what I did in bug 1431957, I have a bunch of patches for this where I have added the necessary using *::*; for every single symbol each separate file requires -- all the easier to be certain I've properly understood what symbols are depended on and that I'm not accidentally bootlegging something from a place I didn't expect. It shouldn't be terribly difficult to go back through at the end and add using namespace js; to cull the js::* symbols that traditionally haven't been singly usingd.

I have a patch stack that takes builtin/Stream.cpp down to 778 lines, of which more than a third are commented-out code that doesn't build at all. The remaining bits seem mostly to be related to byte streams -- which we do not actually support -- and may not even be close to usable, so presently I'm inclined to just leave them there for now and maybe remove them when actual byte stream support gets added. (But in any event, most of the way is far, far better than trying to clear out every last corner of this.)

Also worth noting -- these patches were developed atop a patch that changed FILES_PER_UNIFIED_FILE=1, so these files really should be separately compilable pretty safely.

Priority: -- → P1

Sooooooooooo...as a consequence of bug 1577319 I have learned that our nominal style guide claims that using namespace *; is actually verboten. That bug plans to get rid of all global using namespace std; to solve its problem. But in the longer term, bigger picture, it seems likely that all using namespace *; will disappear -- and if so, it may be in our best interests to keep listing out the distinct usings, rather than me erasing it in a final step.

However -- for now, to reduce the mess of dependencies in my local tree, it's going to be super-helpful for me to be able to land the incremental bits of reviewed code before the whole stack is fully reviewed. And because this namespace thing has some effect on what subsequent revisions touch, I am going to land the patches exactly as they are, without touching anything using namespace immediately. Just a heads-up.

Pushed by jwalden@mit.edu:
https://hg.mozilla.org/integration/autoland/rev/d55a0cb6238a
Rename SetNewList to StoreNewListInFixedSlot, and move it into vm/List-inl.h for use in multiple files.  r=arai
https://hg.mozilla.org/integration/autoland/rev/e6bd168d9b3d
Move queue-with-sizes operations out of builtin/Stream.* into builtin/streams/QueueWithSizes.*.  r=arai
https://hg.mozilla.org/integration/autoland/rev/468ca4f5c8c6
Rename the streams CLASS_SPEC macro to JS_STREAMS_CLASS_SPEC and move it to its own header.  r=arai
https://hg.mozilla.org/integration/autoland/rev/3a7342e03942
Move queueing strategies details into builtin/streams/QueueingStrategies.*.  r=arai
https://hg.mozilla.org/integration/autoland/rev/55e76049ef99
Move various streams miscellaneous operations to a separate file.  r=arai
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Pushed by jwalden@mit.edu:
https://hg.mozilla.org/integration/autoland/rev/724755d6ff65
Move ReadableStream{,Default}Reader details out of builtin/Stream.* to separate files.  r=arai
https://hg.mozilla.org/integration/autoland/rev/a2c5184de226
Move ReadableStreamReader abstract operations out of Stream.cpp.  r=arai
https://hg.mozilla.org/integration/autoland/rev/2b9bdb19e36a
Move public/friend stream API functions into a new file.  r=arai
https://hg.mozilla.org/integration/autoland/rev/3f547590647a
Move TeeState details into separate files.  r=arai
https://hg.mozilla.org/integration/autoland/rev/d8ca7b07183c
Move ReadableStream abstract operations to their own header.  r=arai
https://hg.mozilla.org/integration/autoland/rev/e6c27cc40626
Split PullIntoDescriptor code into separate files.  r=arai
https://hg.mozilla.org/integration/autoland/rev/035b23218006
Move ReadableStream standard library bits into separate files.  r=arai
https://hg.mozilla.org/integration/autoland/rev/b1148ccfa82f
Move IsMaybeWrapped to a header so it can be used in multiple files.  (It can't be *static* in those multiple files because of scumbag unified builds.)  r=arai
https://hg.mozilla.org/integration/autoland/rev/5aa0ce936427
Move ReadableStream stream/controller interactions to separate files.  r=arai
https://hg.mozilla.org/integration/autoland/rev/50eea29e2a96
Move ReadableStreamDefaultController details into separate files.  r=arai
https://hg.mozilla.org/integration/autoland/rev/d42177dc73dc
Move readable stream default controller operations into their own file.  r=arai
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: