Closed Bug 1438670 Opened 6 years ago Closed 6 years ago

Rename gc/Iteration files one more time

Categories

(Core :: JavaScript Engine, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla60
Tracking Status
firefox60 --- fixed

People

(Reporter: jorendorff, Assigned: jorendorff)

Details

Attachments

(2 files)

Bug 1437602 did a couple odd things.

- js/src/gc/Iteration-inl.h was renamed to GCIteration-inl.h, but Iteration.h and Iteration.cpp were not renamed.

- The #ifndef guard in gc/GCIteration-inl.h is gc_GCIteration_h rather than gc_GCIteration_inl_h.

I have a patch that I think fixes both of these; building now...
There are also some comments in the bug mentioning GCIterators.h, but it looks like the actual patch always says GCIteration.h, never GCIterators.h, so I will leave it like that.
Attachment #8951442 - Flags: review?(pbone)
Assignee: nobody → jorendorff
Status: NEW → ASSIGNED
Comment on attachment 8951442 [details] [diff] [review]
Fix gc/GCIteration filenames

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

Thanks for catching this Jason.
Attachment #8951442 - Flags: review?(pbone) → review+
(In reply to Jason Orendorff [:jorendorff] from comment #0)
> - js/src/gc/Iteration-inl.h was renamed to GCIteration-inl.h, but
> Iteration.h and Iteration.cpp were not renamed.

This part was intentional.  Originally the idea was to have GCIteration.h for GC-internal APIs and Iteration.h for APIs used by the rest of the engine.  However, the style checker required me to call the former GCIteration-inl.h since it includes jsgcinlines.h (and eventually gc/ArenaLists-inl.h).  I'm not sure what the best solution is here or what the .cpp file should be called.
Flags: needinfo?(jorendorff)
Attachment #8951646 - Flags: review?(jcoppeard)
Attachment #8951646 - Flags: review?(jcoppeard) → review+
Severity: normal → trivial
Flags: needinfo?(jorendorff)
Priority: -- → P3
Summary: Fix gc/GCIteration filenames → Rename gc/Iteration files one more time
https://hg.mozilla.org/mozilla-central/rev/2dc56cddadcd
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: