Get rid of deprecated nsWritingIterator

RESOLVED FIXED in Firefox 64

Status

()

enhancement
RESOLVED FIXED
8 months ago
8 months ago

People

(Reporter: erahm, Assigned: erahm)

Tracking

unspecified
mozilla64
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox64 fixed)

Details

Attachments

(3 attachments)

+++ This bug was initially created as a clone of Bug #1491494 +++

nsWritingIterator [1] (aka nsTString::iterator [2]) has been deprecated for quite some time. We should remove it and replace usages with nsTStringRepr::char_iterator.

Ideally we want to rename `char_iterator` to `iterator` when the process is done.

[1] https://searchfox.org/mozilla-central/search?q=nsWritingIterator&case=false&regexp=false&path=
[2] https://searchfox.org/mozilla-central/rev/6c82481caa506a240a626bb44a2b8cbe0eedb3a0/xpcom/string/nsTSubstring.h#320
This switches over the few remaining usages of the deprecated
BeginWriting/EndWriting(iterator&) functions to the more standard
BeginWriting/EndWriting() functions.
Attachment #9010000 - Flags: review?(nfroyd)
Assignee: nobody → erahm
Status: NEW → ASSIGNED
It's deprecated and no longer used.
Attachment #9010001 - Flags: review?(nfroyd)
We can just use 'iterator' now that nsWritingIterator is gone.
Attachment #9010036 - Flags: review?(nfroyd)
Attachment #9010000 - Flags: review?(nfroyd) → review+
Comment on attachment 9010001 [details] [diff] [review]
Part 2: Remove nsWritingIterator

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

I vaguely remember trying to transition things in the opposite direction, since a richer interface than pointers enables bounds-checking and things, but I also vaguely remember it being a real slog.  At least with this, we have a single representation, and maybe it's easier to work from there to get something safer.  Thank you!
Attachment #9010001 - Flags: review?(nfroyd) → review+
Attachment #9010036 - Flags: review?(nfroyd) → review+
(In reply to Nathan Froyd [:froydnj] from comment #4)
> Comment on attachment 9010001 [details] [diff] [review]
> Part 2: Remove nsWritingIterator
> 
> Review of attachment 9010001 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I vaguely remember trying to transition things in the opposite direction,
> since a richer interface than pointers enables bounds-checking and things,
> but I also vaguely remember it being a real slog.  At least with this, we
> have a single representation, and maybe it's easier to work from there to
> get something safer.  Thank you!

I the idea was to switch to RangedPtr which would be good for code that just loops over the returned buffer, but for most other cases we'd have to add a call to `get()` to retrieve the raw pointer. Switching over to using writable Spans and the BulkWrite interface might also make sense, but as you noted that'd be a bit of a slog.

Comment 6

8 months ago
Pushed by erahm@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4d671b85f851
Part 1: Stop using deprecated string writing iterators. r=froydnj
https://hg.mozilla.org/integration/mozilla-inbound/rev/1dc310ee7bf7
Part 2: Remove nsWritingIterator. r=froydnj
https://hg.mozilla.org/integration/mozilla-inbound/rev/4a608f94bcff
Part 3: Remove references to char_iterator. r=froydnj

Comment 7

8 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/4d671b85f851
https://hg.mozilla.org/mozilla-central/rev/1dc310ee7bf7
https://hg.mozilla.org/mozilla-central/rev/4a608f94bcff
Status: ASSIGNED → RESOLVED
Last Resolved: 8 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
You need to log in before you can comment on or make changes to this bug.