Conversion to external linkage

[Neil]

In bug 377319, some kind RedHat employee has been looking at converting
the mail back end code to external linkage, but has obviously run in to
a number of issues, some of which don't seem to have been solved, and
others of which have apparently been worked around in a suboptimal way.
I thought I would repeat all of the issues here, in case people can
provide or improve on any or all of the conversions. Note that the code
should compile against both APIs as some people may want to compile
shared or static builds.

Issue: Some functions, such as do_GetService, aren't declared any more
Solution: include the appropriate header, e.g. nsServiceManagerUtils.h

Issue: Can't call Adopt on an nsA(C)String& parameter
For example, return bundle->GetStringFromName(name, getter_Copies(retval));
Possible workaround: Use a local ns(C)String to Adopt and then Assign;
unfortunately this causes an allocation as the returned string cannot
share the temporary string's memory.
Potential solution: Fix the Adopt API in nsStringAPI.h

Issue: The parameters to Find() are different
Workaround: Use #ifdef

Issue: Truncate() does not accept a length argument
Solution: use SetLength instead.

Issue: NS_NewISupportsArray is not available
Solution: use do_CreateInstance(NS_SUPPORTSARRAY_CONTRACTID)
(nsIArray also needs do_CreateInstance anyway.)

Issue: do_QueryElementAt is not available for nsISupportsArray
Workaround: use ->QueryElementAt (which isn't typesafe)
Possible workaround: write CallQueryElementAt
Potential solution: convert the code use nsIArray instead.

Issue: nsString.h does not compile
Solution: use nsStringGlue.h instead.

Issue: LowerCaseEqualsLiteral does not exist
Workaround: use Equals(NS_LITERAL_STRING(...), CaseInsensitiveCompare)

Issue: AppendASCII does not exist
Workaround: use Append(NS_LITERAL_STRING(...))
Potential solution: Add it to nsStringAPI.h (which ironically does
provide AppendLiteral).

Issue: SetCharAt does not exist
Workaround: write MsgSetCharAt method
Potential solution: Add it to nsStringAPI.h

Issue: Right does not exist
Issue: Left does not exist
Solution: Use StringHead, StringTail or Substring instead.

Issue: NS_CopyUnicodeToNative does not exist
Issue: NS_CopyNativeToUnicode does not exist
Potential solution: Use the Unicode nsILocalFile methods directly.

Issue: kNotFound is not defined
Solution: Use -1

Issue: NS_GetCurrentThread is not defined
Workaround/solution: Use nsCOMPtr<nsIThread> thread(do_GetCurrentThread());

Issue: ToInteger takes a different type parameter
Workaround: Use #ifdef to declare the parameter
Potential solution: Fix nsTString to accept an nsresult result(!).

Issue: Last does not exist
Workaround: Use EndReading()[-1]
Potential solution: Add Last to nsStringAPI.h

Issue: SetCapacity does not exist
Workaround: Don't use it, or #ifdef it

Issue: The following functions do not exist or are uncallable from
external linkage:
ReplaceChar ReplaceSubstring FindCharInSet NS_RegisterStaticAtoms
NS_NewAtom nsEscape nsUnescape NS_NewCStringInputStream nsEscapeHtml2
NS_UnescapeURL NS_NewInterfaceRequestorAggregation IsASCII IsUTF8
NS_NewAdoptingUTF8StringEnumerator
NS_NewNotificationCallbacksAggregation NS_GetProxyForObject SortIgnoreCase
Workaround: For some of these a private MsgXXX function was written.
However there may be other ways of calling those functions of which
neither of us are aware.

Issue: + (string concatenation) does not exist
Workaround: Use Assign and Append

Issue: Append<X>to<Y> does not exist
Workaround: Use Append with NS_Convert<X>to<Y>

Issue: ToNewCString does not work on an nsAString
Solution: use a helper NS_LossyConvertUTF16toASCII

Issue: Recycle does not exist
Solution: use NS_Free, or rewrite the code using ns(C)String.Adopt instead

Issue: UTF8ToNewUnicode does not exist
Solution: use a helper NS_ConvertUTF8toUTF16

--
Warning: May contain traces of nuts.

[Robert Kaiser]

Neil schrieb:

> Note that the code
> should compile against both APIs as some people may want to compile
> shared or static builds.

I was under the impression that compiling against both APIs was just a
temporary measure until we can set external API by default and remove
internal API support (after some baking time). Am I wrong with that
impression?

Robert Kaiser

[Neil]

Robert Kaiser wrote:

My understanding is that each module must be internally consistent as to
which API it uses, so if you want to continue to support static builds
that include mail then it still needs to compile with the internal API.
Also people compiling shared builds would prefer to use the internal API
as it will perform better and is easier to debug.

[Benjamin Smedberg]

On 12/29/09 4:35 PM, Neil wrote:
>> I was under the impression that compiling against both APIs was just a
>> temporary measure until we can set external API by default and remove
>> internal API support (after some baking time). Am I wrong with that
>> impression?
>
> My understanding is that each module must be internally consistent as to
> which API it uses, so if you want to continue to support static builds
> that include mail then it still needs to compile with the internal API.
> Also people compiling shared builds would prefer to use the internal API
> as it will perform better and is easier to debug.

It is currently true that you should not mix the internal and external
string APIs, because there are symbols shared between them such as
nsString::nsString. That could be fixed by using namespaces, e.g.
mozilla::internal::nsAString / mozilla::glue::nsAString, and the headers
would choose which version to use with `using` declarations and typedefs.
This would also get rid of the rather unholy nsAString_internal #define that
we currently have and avoid all of the symbol conflicts.

--BDS

[Robert Kaiser]

Hmm, I'm not too good at the details of those linkage things when it
comes down to how code is written.
From a build perspective, I'd favor going to libxul or even XULRunner
in the long term, matching what Firefox does as much as possible. I
guess mail would need to be built in the same way as Firefox does with
their app-specific modules.
I believe the old shared and static build targets will disappear one
time, IIRC Firefox doesn't allow any of them any more and we should
follow that as well.
I somehow hope that debug builds will continue to not need linking
really huge libraries, but I mostly hope we can get as near to Firefox
in how we're building stuff as possible - e.g. we need to build libxul
if we want OOPP, AFAIK, and SeaMonkey really wants to enable that one
time, I guess.

Robert Kaiser

[Neil]

Neil wrote:

> Issue: SetCharAt does not exist

I've just realised that I can replace(!) this with a call to Replace. No
takers for the other methods though...

[Neil]

Neil wrote:

> Issue: Can't call Adopt on an nsA(C)String& parameter
> For example, return bundle->GetStringFromName(name,
> getter_Copies(retval));
> Possible workaround: Use a local ns(C)String to Adopt and then Assign;
> unfortunately this causes an allocation as the returned string cannot
> share the temporary string's memory.
> Potential solution: Fix the Adopt API in nsStringAPI.h

So it seems that this is a non-starter because
NS_(C)StringContainerInit2 only works (oddly enough) on an
ns(C)StringContainer, which under internal linkage nsA(C)String
effectively is, but under external linkage nsA(C)String is not.

Is there any particular reason for the distinction?

[Benjamin Smedberg]

On 1/4/10 4:20 PM, Neil wrote:

> So it seems that this is a non-starter because
> NS_(C)StringContainerInit2 only works (oddly enough) on an
> ns(C)StringContainer, which under internal linkage nsA(C)String
> effectively is, but under external linkage nsA(C)String is not.
>
> Is there any particular reason for the distinction?

AIUI, nsStringContainer is equivalent to nsString, which is always
null-terminated, but nsAString is not.

--BDS

[Neil]

Benjamin Smedberg wrote:

Oh well, in that case I guess we're always going to have to copy wstring
return values into AString parameters.

[Neil]

Neil wrote:

> In bug 377319, some kind RedHat employee has been looking at
> converting the mail back end code to external linkage, but has
> obviously run in to a number of issues, some of which don't seem to
> have been solved, and others of which have apparently been worked
> around in a suboptimal way. I thought I would repeat all of the issues
> here, in case people can provide or improve on any or all of the
> conversions. Note that the code should compile against both APIs as
> some people may want to compile shared or static builds.

One I overlooked: CountChar does not exist. But we could rewrite the
logic not to require it.

[Neil]

Benjamin Smedberg wrote:

Except nsDependent(C)Substring also extends ns(C)StringContainer, so
that isn't even true...

[Neil]

Neil wrote:

> Issue: NS_CopyUnicodeToNative does not exist
> Issue: NS_CopyNativeToUnicode does not exist
> Potential solution: Use the Unicode nsILocalFile methods directly.

Potential solution: nsXPCOMStrings actually supports
NS_CSTRING_ENCODING_NATIVE_FILESYSTEM, so the two methods could be added
to nsStringAPI.h

[Neil]

Neil wrote:

Of course, that's not the problem. The problem is that
NS_(C)StringContainerInit has to work on an ns(C)StringContainer, but
NS_(C)StringGetData has to work on an nsA(C)String, which means that
ns(C)StringContainer has to extend nsA(C)String. Which means that the
only way that I can Adopt into an nsA(C)String& is to static cast it to
at least an ns(C)StringContainer first...

How big a change would it be for NS_(C)StringContainerInit to take an
nsA(C)String& instead?

[Benjamin Smedberg]

On 1/10/10 4:41 PM, Neil wrote:

> How big a change would it be for NS_(C)StringContainerInit to take an
> nsA(C)String& instead?

Well... currently we pretend that you could use nsXPCOMStrings.h without any
reference to nsStringAPI.h, and that it's a C API (not C++).

It's likely that the distinction is not worth keeping and we should just
make this entire thing a C++ API and have nsAString at the root (and forget
nsStringContainer entirely).

--BDS