If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Remove unnecessary RangedPtr includes

RESOLVED FIXED in mozilla23

Status

()

Core
JavaScript Engine
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: dholbert, Assigned: dholbert)

Tracking

Trunk
mozilla23
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

5 years ago
In doing a little research on RangedPtr, I noticed several files that #include it without using it, all in the /js/src directory.

Filing this bug on removing those #includes.
(Assignee)

Comment 1

5 years ago
The files in question are:
 * jsarray.cpp
    - #include was added in https://hg.mozilla.org/mozilla-central/rev/570ff6efe5ef#l9.16 for bug 668024, apparently accidentally. (Nothing in that commit uses RangedPtr)

 * jsfun.cpp
     - #include was added in https://hg.mozilla.org/mozilla-central/rev/e080642175e6#l39.12 for Bug 761723.
     - That patch's RangedPtr usage was later replaced with StableCharPtr in bug 761723

 * jsonparser.h
     - #include was added in https://hg.mozilla.org/mozilla-central/rev/739c0fd21ccc#l3.12
      - That ptach's RangedPtr usage was later replaced with StableCharPtr in bug 798624
Depends on: 668024, 761723
(Assignee)

Updated

5 years ago
Depends on: 662001, 798624
(Assignee)

Comment 2

5 years ago
Created attachment 718865 [details] [diff] [review]
fix
Attachment #718865 - Flags: review?(jwalden+bmo)
(Assignee)

Updated

5 years ago
Assignee: general → dholbert
Status: NEW → ASSIGNED
Attachment #718865 - Flags: review?(jwalden+bmo) → review+
(Assignee)

Comment 3

5 years ago
Just rediscovered this bug -- apparently I forgot to land its patch.

Looks like the first chunk, for jsarray.cpp, was fixed in bug 634839 part 1:
 https://hg.mozilla.org/mozilla-central/rev/9ab1119d4596#l6.1

The other two chunks are still valid, though.
(Assignee)

Comment 4

5 years ago
RE the jsonparser.h tweak: jsonparser.cpp (not .h) *does* actually have some RangedPtr usage, so it technically should be including RangedPtr.

It gets it for free from jsapi, so it builds fine regardless, but it'd be better to include it directly rather than depending on a several-levels-removed #include.
(Assignee)

Comment 5

5 years ago
Created attachment 744891 [details] [diff] [review]
fix v2: rebased, and shifting the jsonparser #include to the .cpp file

Tagging Waldo for review on the jsonparser.cpp tweak. (adding the #include there)

(For completeness, the RangedPtr usages there are:
 http://mxr.mozilla.org/mozilla-central/source/js/src/jsonparser.cpp#207
and
 http://mxr.mozilla.org/mozilla-central/source/js/src/jsonparser.cpp#389
)
Attachment #718865 - Attachment is obsolete: true
Attachment #744891 - Flags: review?(jwalden+bmo)
Attachment #744891 - Flags: review?(jwalden+bmo) → review+
(Assignee)

Comment 6

4 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/47ad8bdaae94
Flags: in-testsuite-
https://hg.mozilla.org/mozilla-central/rev/47ad8bdaae94
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
You need to log in before you can comment on or make changes to this bug.