1374580 - Remove useless string typedefs

Status: RESOLVED FIXED

(Core :: XPCOM, enhancement)

Description

[Nicholas Nethercote [inactive]]

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.

Comment 1

[Nicholas Nethercote [inactive]]

Attachment: (part 1) - Remove nsASingleFragment{,C}String typedefs

All the instances are converted as follows.

- nsASingleFragmentString  --> nsAString
- nsASingleFragmentCString --> nsACString

Comment 2

[Nicholas Nethercote [inactive]]

Attachment: (part 2) - Remove nsAFlat{,C}String typedefs

All the instances are converted as follows.

- nsAFlatString  --> nsString
- nsAFlatCString --> nsCString

Comment 3

[Nicholas Nethercote [inactive]]

Attachment: (part 3) - Remove ns{,C}Substring typedefs

All the instances are converted as follows.

- nsSubstring  --> nsAString
- nsCSubstring --> nsACString

Comment 4

[Nicholas Nethercote [inactive]]

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

Comment 5

[Nathan Froyd [:froydnj]]

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.

Comment 6

[Nicholas Nethercote [inactive]]

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

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/87b31f17dd99
https://hg.mozilla.org/mozilla-central/rev/2915d4b78496
https://hg.mozilla.org/mozilla-central/rev/6352096eb0de

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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.

Comment 9

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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.)

Comment 10

[Nicholas Nethercote [inactive]]

(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.