Remove useless string typedefs

RESOLVED FIXED in Firefox 56

Status

()

enhancement
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: njn, Assigned: njn)

Tracking

Trunk
mozilla56
Points:
---

Firefox Tracking Flags

(firefox56 fixed)

Details

Attachments

(3 attachments)

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.
All the instances are converted as follows.

- nsASingleFragmentString  --> nsAString
- nsASingleFragmentCString --> nsACString
Attachment #8879475 - Flags: review?(nfroyd)
Assignee: nobody → n.nethercote
Status: NEW → ASSIGNED
All the instances are converted as follows.

- nsAFlatString  --> nsString
- nsAFlatCString --> nsCString
Attachment #8879476 - Flags: review?(nfroyd)
All the instances are converted as follows.

- nsSubstring  --> nsAString
- nsCSubstring --> nsACString
Attachment #8879477 - Flags: review?(nfroyd)
Attachment #8879475 - Flags: review?(nfroyd) → review+
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+
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
Closed: 2 years ago
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)
(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.