Closed Bug 53057 Opened 24 years ago Closed 16 years ago

[API] turn off implicit |CharT*| conversion operators for those string classes that have them

Categories

(Core :: XPCOM, defect, P3)

defect

Tracking

()

RESOLVED FIXED
Future

People

(Reporter: scc, Assigned: jag+mozilla)

References

Details

Attachments

(11 files)

(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), text/plain
Details
(deleted), patch
jag+mozilla
: review+
Details | Diff | Splinter Review
I added |.get()| to replace this with an explicit form; went through the tree and fixed places where people rely on the implicit form; now just need to remove the actual implicit operators. Why? Implicit conversion repeatedly leads us down the garden patch to calling wrong functions and doing more work than the caller expected often ending in multiple copies and conversions of the string.
Status: NEW → ASSIGNED
the patch is a little stale; I'll have to go through and re-find the stragglers, but this is important.
Component: XPCOM → String
Target Milestone: --- → mozilla0.9
Cool. Perhaps I should do this so dbaron can r= and you can a=?
That would be great!
So far only the issues which turned up in my compile (which doesn't include any tests nor most of the extensions currently). I'll try and do the rest tomorrow. Assigning to me so I can find it in my buglist :-)
Assignee: scc → disttsc
Status: ASSIGNED → NEW
xpccomponents.cpp and xpcexception.cpp have both been fixed in another bug.
dbaron, would you mind reviewing this sometime soon? More get-less literal strings keep sneaking in :-)
basic_nsLiteralString even. With the other patches applied I could safely comment that operator out.
r=timeless for 24530,24907
more and more, we see how important this is.
Blocks: 67961
Status: NEW → ASSIGNED
Blocks: 48853
Depends on: 70075
Depends on: 70090
No longer depends on: 70090
Blocks: 70090
Target Milestone: mozilla0.9 → mozilla0.8.1
sr=scc
r=alecf on that last patch I would like to voice my objection to the global Compare() and ToNewUnicode() functions, but that can be tackled another time
So this compiles on linux, windows and mac. Anyone care to review? scc, sr=? I'm hoping to get this one in for 0.8.1 as targetted. I _doubt_ I'll get the (large!) |nsCString::operator const char*() const| removal patch in though (in part because I still need to create it).
The patch looks like safe, mechanical progress. I"m glad _I_ didn't have to type all those filthy NS_CONST_CASTs, lemme tell ya. =) sr=shaver
At some point, it would be nice if nsLiteralCString and friends could be used in some of the instances where the constness is just being cast away in order to insert the const char * into an nsCString. But this is already an excellent step forward. r=dmose@netscape.com
Never mind my cleanup comment; that was me misinterpreting the change to nsAddrBookUrl::CrackPrintUrl(). r=dmose.
Moving to 0.9
Target Milestone: mozilla0.8.1 → mozilla0.9
Stuff checked in... Next up: operator const char*() const.
Bah
Target Milestone: mozilla0.9 → mozilla0.9.1
Summary: turn off implicit |CharT*| conversion operators for those string classes that have them → [API] turn off implicit |CharT*| conversion operators for those string classes that have them
Next one then
Target Milestone: mozilla0.9.1 → mozilla0.9.2
Assignee: jag → jaggernaut
Status: ASSIGNED → NEW
->0.9.3, not a limbo candidate though
Target Milestone: mozilla0.9.2 → mozilla0.9.3
Target Milestone: mozilla0.9.3 → mozilla0.9.4
> nsJSEnvironment.cpp: > > Please use NS_REINTERPRET_CAST. Or maybe we need inline functions > that are PRtoJSUnichar and JStoPRUnichar (or better names) and do > the reinterpret cast within and are externally typesafe? That would > probably be a good thing if you can find a good place to put them... No good place found yet. nsJSUtils.h, nsWWJSUtils.h and some new file for xpinstall/*? Or perhaps a new file in string/public? > nsHTMLEditRules.cpp, nsTextEditRules.cpp: > > ***I need to go over these changes again.*** > inBitmapURI.cpp : > > Are ContractIDs really preferred to CIDs in C++ code? Yes, as far as I know they are. > inBitmapURI::FormatSpec could really be written better, but you don't > have to change it if you don't want... Done :-) > COtherElements.h : > > No need to split to 2 lines. - const char* tag=nsHTMLTags::GetStringValue(aTag->mTag); + const nsCString& tagStr=nsHTMLTags::GetStringValue(aTag->mTag); + const char* tag=tagStr.get(); I guess you're right, but I figured this would make it clearer that you'll need the nsCString to stay around for the duration of time you need |tag|. > TestCSSPropertyLookup.cpp : > You should be able to use nsDependentString for the first. I'd have to convert to UCS2 for that, and there's no support for nsDependentCString (yet). > mailnews/import/eudora/src/nsEudoraAddress.cpp > > Are you in a position to remove the pCStr variable? Yep, done. > mailnews/mime/src/mimetpla.cpp > > ***I need to go over these changes again.*** > > No need to create an nsDependentString around an nsXPIDLString > ("nsDependentString(citeTagsResultUnichar)"). > > Use |Adopt| rather than |*getter_Copies() =|. This code was from before the "recent" changes to nsXPIDLString. Changes made now though :-) > modules/libpr0n/decoders/icon/nsIconURI.cpp > > Could you leave the CID? They're not deprecated, are they? No, but I'm pretty sure Contract IDs are prefered over Class IDs in most cases. I can't remember who's stated that though, so let me find out. > netwerk/mime/src/nsMIMEInfoImpl.cpp > > ***I need to go over these changes again.*** > nsString2.{h,cpp} > > ***I need to go over these changes again.*** > xpcom/components/nsComponentManager.cpp > > Why don't you fix the longstanding bug here and make it > entry->location.Equals(registryName) ? Done. > xpfe/bootstrap/nsAppRunner.cpp > > I assume these changes were all for a different bug. Oops, yes.
0.9.5
Target Milestone: mozilla0.9.4 → mozilla0.9.5
Mass-moving lower-priority 0.9.5 bugs off to 0.9.6 to make way for remaining 0.9.4/eMojo bugs, and MachV planning, performance and feature work. If you disagree with any of these targets, please let me know.
Target Milestone: mozilla0.9.5 → mozilla0.9.6
-> 1.0
Target Milestone: mozilla0.9.6 → mozilla1.0
Almost done with this one. -> 0.9.7
Target Milestone: mozilla1.0 → mozilla0.9.7
Checked in the removal of |operator const char*() const| from nsCString. Do we want to do nsXPIDLString and nsXPIDLCString?
It looks that the latest checkin shot XLib toolkit dead: -- snip -- nsDragService.cpp Building deps for ../../../../../../../../home/mozilla/src/2001-11-14-08-trunk/mozilla/widget/src/xlib/nsDragService.cpp /opt/SUNWspro/bin/CC -xarch=v9 -o nsDragService.o -c -DOSTYPE=\"SunOS5\" -DOSARCH=\"SunOS\" -DOJI -D_IMPL_NS_WIDGET -I../../../../../../../../home/mozilla/src/2001-11-14-08-trunk/mozilla/widget/src/xlib/../xpwidgets -I../../../../../../../../home/mozilla/src/2001-11-14-08-trunk/mozilla/widget/src/xlib -I../../../dist/include/xpcom -I../../../dist/include/string -I../../../dist/include/appshell -I../../../dist/include/gfx -I../../../dist/include/layout -I../../../dist/include/content -I../../../dist/include/dom -I../../../dist/include/js -I../../../dist/include/xlibrgb -I../../../dist/include/timer -I../../../dist/include/uconv -I../../../dist/include/pref -I../../../dist/include/webshell -I../../../dist/include/htmlparser -I../../../dist/include/view -I../../../dist/include/necko -I../../../dist/include/gfx2 -I../../../dist/include/widget -I../../../dist/include -I/shared/bigtmp2/mozilla/2001-11-14-08-trunk/objdir_ws6_xlib_sparcv9/dist/include/nspr -I/usr/openwin/include -KPIC -I/usr/openwin/include -mt -O -DDEBUG -DDEBUG_mozilla -DTRACING -g -I/usr/openwin/include -I/usr/openwin/include -DMOZILLA_CLIENT -DBROKEN_QSORT=1 -DNSCAP_DISABLE_DEBUG_PTR_TYPES=1 -DD_INO=d_ino -DSTDC_HEADERS=1 -DHAVE_ST_BLKSIZE=1 -DHAVE_INT16_T=1 -DHAVE_INT32_T=1 -DHAVE_INT64_T=1 -DHAVE_UINT=1 -DHAVE_UINT_T=1 -DHAVE_UINT16_T=1 -DHAVE_64BIT_OS=1 -DHAVE_DIRENT_H=1 -DHAVE_SYS_BYTEORDER_H=1 -DHAVE_MEMORY_H=1 -DHAVE_UNISTD_H=1 -DHAVE_NL_TYPES_H=1 -DHAVE_SYS_STATVFS_H=1 -DHAVE_SYS_STATFS_H=1 -DHAVE_SYS_VFS_H=1 -DHAVE_SYS_MOUNT_H=1 -DHAVE_LIBM=1 -DHAVE_LIBDL=1 -DHAVE_LIBSOCKET=1 -D_REENTRANT=1 -DHAVE_RANDOM=1 -DHAVE_STRERROR=1 -DHAVE_LCHOWN=1 -DHAVE_FCHMOD=1 -DHAVE_SNPRINTF=1 -DHAVE_LOCALTIME_R=1 -DHAVE_STATVFS=1 -DHAVE_MEMMOVE=1 -DHAVE_RINT=1 -DHAVE_NL_LANGINFO=1 -DHAVE_STRTOK_R=1 -DHAVE_IOS_BINARY=1 -DHAVE_CPP_EXPLICIT=1 -DHAVE_CPP_SPECIALIZATION=1 -DHAVE_CPP_MODERN_SPECIALIZE_TEMPLATE_SYNTAX=1 -DHAVE_CPP_PARTIAL_SPECIALIZATION=1 -DHAVE_CPP_ACCESS_CHANGING_USING=1 -DHAVE_CPP_AMBIGUITY_RESOLVING_USING=1 -DHAVE_CPP_NAMESPACE_STD=1 -DHAVE_CPP_UNAMBIGUOUS_STD_NOTEQUAL=1 -DHAVE_CPP_NEW_CASTS=1 -DHAVE_CPP_DYNAMIC_CAST_TO_VOID_PTR=1 -DNEED_CPP_UNUSED_IMPLEMENTATIONS=1 -DHAVE_I18N_LC_MESSAGES=1 -DMOZ_DEFAULT_TOOLKIT=\"xlib\" -DMOZ_WIDGET_XLIB=1 -DMOZ_ENABLE_XREMOTE=1 -DMOZ_X11=1 -DIBMBIDI=1 -DSUNCTL=1 -DACCESSIBILITY=1 -DMOZ_MATHML=1 -DMOZ_SVG=1 -DMOZ_LOGGING=1 -DDETECT_WEBSHELL_LEAKS=1 -DMOZ_USER_DIR=\".mozilla\" -DMOZ_XUL=1 -DINCLUDE_XUL=1 -DNS_MT_SUPPORTED=1 -DUSE_IMG2=1 -DMOZ_DLL_SUFFIX=\".so\" -DXP_UNIX=1 -DUNIX_ASYNC_DNS=1 -DJS_THREADSAFE=1 -DNS_PRINT_PREVIEW=1 -DMOZ_REFLOW_PERF=1 -DMOZ_REFLOW_PERF_DSP=1 ../../../../../../../../home/mozilla/src/2001-11-14-08-trunk/mozilla/widget/src/xlib/nsDragService.cpp "../../../../../../../../home/mozilla/src/2001-11-14-08-trunk/mozilla/widget/src/xlib/nsDragService.cpp", line 212: Error: Cannot cast from nsCAutoString to const char*. "../../../../../../../../home/mozilla/src/2001-11-14-08-trunk/mozilla/widget/src/xlib/nsDragService.cpp", line 213: Error: Cannot cast from nsCAutoString to const char*. 2 Error(s) detected. -- snip -- I'll file a fix for that ...
Can someone r= the patch and check it in, please ?
Comment on attachment 57924 [details] [diff] [review] Fix for the Xlib toolkit bustage (2001-11-14-08-trunk) r=jag
Attachment #57924 - Flags: review+
Need to discuss what to do about nsXPIDLC?String with scc and dbaron. Moving to 0.9.since this will most likely not get fixed in 0.9.7
Target Milestone: mozilla0.9.7 → mozilla0.9.9
Blocks: 104723
-> future
Target Milestone: mozilla0.9.9 → Future
filter keyword: nothingnessless
operator const char_type* has been removed for everything but nsXPIDLC?String. Going to mark this bug fixed. I'll see about filing a new bug to deal with that case.
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Component: String → XPCOM
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: