Closed Bug 914032 Opened 6 years ago Closed 6 years ago

Slim down even more -inl.h files

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla26

People

(Reporter: njn, Assigned: njn)

Details

(Whiteboard: [js:t])

Attachments

(3 files)

This follows on from bug 910771.
vm/Shape.h currently depends on vm/ObjectImpl.h.  This patch inverts this
dependence.  I had to move a few things (most notable PropDesc.h) from the
latter to the former, but it's mostly straightforward.
Attachment #801469 - Flags: review?(terrence)
This is nice.  Two -inl.h files bite the dust, as do several |uninlined*()|
functions, and vm/ObjectImpl.h roughly halves in size.
Attachment #801472 - Flags: review?(terrence)
Lots of barrier movement here.  These functions are all so repetitive, I'm sure
they could be templatized.  But not right now.
Attachment #801473 - Flags: review?(terrence)
Comment on attachment 801469 [details] [diff] [review]
(part 1) - Invert the dependency between vm/Shape.h and vm/ObjectImpl.h.

Review of attachment 801469 [details] [diff] [review]:
-----------------------------------------------------------------

Nice! r=me
Attachment #801469 - Flags: review?(terrence) → review+
Comment on attachment 801472 [details] [diff] [review]
(part 2) - Move a bunch of stuff out of -inl.h files.

Review of attachment 801472 [details] [diff] [review]:
-----------------------------------------------------------------

The dependency inversion between shape and object has always bugged me, precisely because it prevents us from using lastProperty in the header. Good to know that is actually as significant a cleanup as I imagined. r=me

::: js/src/gc/Barrier.cpp
@@ +50,5 @@
>  }
>  
>  #endif // DEBUG
>  
> +#ifdef DEBUG

You can probably merge these two adjacent DEBUG sections.
Attachment #801472 - Flags: review?(terrence) → review+
Comment on attachment 801473 [details] [diff] [review]
(part 3) - Move a bunch more stuff out of -inl.h files.

Review of attachment 801473 [details] [diff] [review]:
-----------------------------------------------------------------

Yes, the pre-barriers in particular are pretty much uniform. r=me
Attachment #801473 - Flags: review?(terrence) → review+
https://hg.mozilla.org/mozilla-central/rev/c8175c00be1e
https://hg.mozilla.org/mozilla-central/rev/ea1af870680c
https://hg.mozilla.org/mozilla-central/rev/ea33604f6232
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
You need to log in before you can comment on or make changes to this bug.