717356 - String classes should achieve type genericity using templates, not macros

Status: RESOLVED INCOMPLETE

(Core :: XPCOM, defect)

Description

[Benoit Jacob [:bjacob] (mostly away)]

Attachment: templates++, macros--

See e.g. nsTSubstring_CharT in nsTSubstring.h.

I'm attaching a patch that's how far I went before deciding to stop this work as I'm not sure I understand everything that I should.

The basic design choice that I can see here, is whether e.g. nsString and nsCString should just be typedefs for specializations of the same nsTString template, or should they rather be classes of their own, inheriting specializations of a common base class template nsTString (maybe call it nsTStringBase).

The present patch implements the typedef approach. For example, nsTString_CharT is replaced by a nsTString class template and then nsString is a typedef for nsTString<PRUnichar>. The advantage of this typedef approach is that it avoids uselessly increasing the number of types at hand. It's easy to wrap one's head around the notion that nsString is a typedef for nsTString<PRUnichar>, it just makes sense. The downside is that it makes it harder to have specific methods in only one of nsString and nsCString and not in the other. That's where I stopped: if you try compiling with this patch, you'll get compile errors as nsStringObsolete.cpp tries to define methods specifically in nsString / in nsCString, and I wasn't sure what to do about that. In itself it sounds legitimate to have specific methods, but as a matter of fact, the only such type-specific methods we have are "obsolete" (whatever that means --- they're still enabled).

By default I'm leaving it here, unless someone in the know thinks this is worth carrying on and explains how to best deal with these obsolete methods.

Comment 1

[Benjamin Smedberg]

Darin tried this originally and moved "back" to the templates because the minor API differences and the somewhat major implementation differences meant that you had to basically specialize everything, which removes most of the benefit.

I understand that the templates give somewhat better code readability/searchability, but I don't think it's worth the effort in general to change things yet again.

Comment 2

[Benoit Jacob [:bjacob] (mostly away)]

There's something that I don't understand here: while not everything can be made generic, I was thinking that what is made generic with macros could equally be made generic with templates. That part works nicely in above patch.

Anyway, don't want to steal more of your time.

Comment 3

[Benoit Jacob [:bjacob] (mostly away)]

Oops, I didn't mean to reopen this bug.