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

Remove useless string typedefs

RESOLVED FIXED in Firefox 56

Status

()

Core
String
RESOLVED FIXED
3 months ago
3 months ago

People

(Reporter: njn, Assigned: njn)

Tracking

Trunk
mozilla56
Points:
---

Firefox Tracking Flags

(firefox56 fixed)

Details

Attachments

(3 attachments)

(Assignee)

Description

3 months ago
nsStringFwd.h defines the following typedefs:

- ns{,C}Substring
- nsASingleFragment{,C}String
- nsAFlat{,C}String

They all map to either ns{,C}String or nsA{,C}String.

They are just needless indirection, and confusing. Let's get rid of them.

I will update https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM/Guide/Internal_strings once the patches land.
(Assignee)

Comment 1

3 months ago
Created attachment 8879475 [details] [diff] [review]
(part 1) - Remove nsASingleFragment{,C}String typedefs

All the instances are converted as follows.

- nsASingleFragmentString  --> nsAString
- nsASingleFragmentCString --> nsACString
Attachment #8879475 - Flags: review?(nfroyd)
(Assignee)

Updated

3 months ago
Assignee: nobody → n.nethercote
Status: NEW → ASSIGNED
(Assignee)

Comment 2

3 months ago
Created attachment 8879476 [details] [diff] [review]
(part 2) - Remove nsAFlat{,C}String typedefs

All the instances are converted as follows.

- nsAFlatString  --> nsString
- nsAFlatCString --> nsCString
Attachment #8879476 - Flags: review?(nfroyd)
(Assignee)

Comment 3

3 months ago
Created attachment 8879477 [details] [diff] [review]
(part 3) - Remove ns{,C}Substring typedefs

All the instances are converted as follows.

- nsSubstring  --> nsAString
- nsCSubstring --> nsACString
Attachment #8879477 - Flags: review?(nfroyd)
(Assignee)

Comment 4

3 months ago
Try looks good: https://treeherder.mozilla.org/#/jobs?repo=try&revision=78810653cc46f6090519904feb616411d1c32bc7

Updated

3 months ago
Attachment #8879475 - Flags: review?(nfroyd) → review+

Updated

3 months ago
Attachment #8879476 - Flags: review?(nfroyd) → review+
Comment on attachment 8879477 [details] [diff] [review]
(part 3) - Remove ns{,C}Substring typedefs

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

Consistency is a good thing.
Attachment #8879477 - Flags: review?(nfroyd) → review+
(Assignee)

Comment 6

3 months ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/87b31f17dd993477d662161290335e01844530ed
Bug 1374580 (part 1) - Remove nsASingleFragment{,C}String typedefs. r=froydnj.

https://hg.mozilla.org/integration/mozilla-inbound/rev/2915d4b7849612e583048650beabe33bad2cfeca
Bug 1374580 (part 2) - Remove nsAFlat{,C}String typedefs. r=froydnj.

https://hg.mozilla.org/integration/mozilla-inbound/rev/6352096eb0de303cba9440092279e4254a1ec586
Bug 1374580 (part 3) - Remove ns{,C}Substring typedefs. r=froydnj.

Comment 7

3 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/87b31f17dd99
https://hg.mozilla.org/mozilla-central/rev/2915d4b78496
https://hg.mozilla.org/mozilla-central/rev/6352096eb0de
Status: ASSIGNED → RESOLVED
Last Resolved: 3 months ago
status-firefox56: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
For what it's worth, I'd have far preferred removing the nsAString name in favor of nsSubstring.  It's no longer an abstract type, but the String/Substring distinction is the key one.
Component: XPCOM → String
I'm inclined to ask that you revert part 3 here, because I really think we should go in the other direction and standardize on the ns{,C}Substring names.  But perhaps we should discuss next week at the all-hands?

(We had some previous discussion of this in bug 1346078.)
Flags: needinfo?(n.nethercote)
(Assignee)

Comment 10

3 months ago
(In reply to David Baron :dbaron: ⌚️UTC-7 from comment #9)
> I'm inclined to ask that you revert part 3 here, because I really think we
> should go in the other direction and standardize on the ns{,C}Substring
> names.  But perhaps we should discuss next week at the all-hands?

I'm happy to discuss more. In case it's helpful, froydnj and I discussed on IRC and here is the reasoning.

- I did some quick measurements and the use of nsA[C]String was about 50x more common than the use of ns[C]Substring. I.e. 10s of thousands of uses vs hundreds.

- Combine that with the anticipated difficulty of changing XPIDL (which dmajor confirms in bug 1346078 comment 14).

- Also add in some additional VCS blame churn.

And removing ns[C]Substring felt like the right thing to do. Even if nsA[C]String isn't a great name, it's a very well established one.
Flags: needinfo?(n.nethercote)
You need to log in before you can comment on or make changes to this bug.