The default bug view has changed. See this FAQ.

remove pitfall nsTString_CharT(char_type c) constructor

RESOLVED FIXED in mozilla11

Status

()

Core
String
RESOLVED FIXED
7 years ago
5 years ago

People

(Reporter: vlad, Assigned: Atul Aggarwal)

Tracking

unspecified
mozilla11
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

This constructor is a great pitfall, because it's very tempting to write code like this:

  int len = ...;
  ...
  nsCAutoString str(len);
  SomeMethod(str.BeginWriting());

  str(len) doesn't even show a warning, which I don't quite understand, since it's a narrower type.  Would be nice to run an analysis to see where the (char) form is used and if we can just get rid of them, then just get rid of this constructor -- or perhaps make it do the more useful thing (capacity len, length 0).
I just tried commenting this ctor out, as well as making it private, and both compile fine on debug Linux.  Want to just do a tryserver run with it private or removed, and if that passes push it?

Comment 2

7 years ago
It's used by xpcom/tests/TestTArray.cpp but that appears to only be included in the Makefile in a disable libxul build.
(Reporter)

Comment 3

7 years ago
So if it's not actually used -- what about actually making it do the more useful thing, making it equivalent to str.SetCapacity(len) ?  Though you'd still need to remember to call SetLength after writing to it, so maybe just getting rid of it is best.

I'll make a patch and fix up TestTArray.cpp.
(Assignee)

Comment 4

5 years ago
Created attachment 574912 [details] [diff] [review]
Patch v1

Removing the culprit ctor. I didn't get any failure so I am assuming TestTArray.cpp file has changed.
https://tbpl.mozilla.org/?tree=Try&rev=2d768e29de54
Attachment #574912 - Flags: review?(dbaron)
Comment on attachment 574912 [details] [diff] [review]
Patch v1

r=dbaron
Attachment #574912 - Flags: review?(dbaron) → review+
(Assignee)

Updated

5 years ago
Keywords: checkin-needed
Assignee: nobody → atulagrwl
https://hg.mozilla.org/integration/mozilla-inbound/rev/346e090f5a4d
Flags: in-testsuite-
Keywords: checkin-needed
Target Milestone: --- → mozilla11
https://hg.mozilla.org/mozilla-central/rev/346e090f5a4d
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.