1499212 - issues with comments re "Literal" methods

Status: RESOLVED FIXED

(Core :: XPCOM, enhancement, P3)

Description

[Karl Tomlinson (:karlt)]

+++ This bug was initially created as a clone of Bug #326317 +++

This bug tracks similar changes to those made in
https://hg.mozilla.org/mozilla-central/rev/f7b1ac19bfac
as well as a few other clarifications.

Comment 1

[Karl Tomlinson (:karlt)]

Attachment: Bug 1499212 remove mention of character pointers in docs for Literal methods r?dbaron

Since https://hg.mozilla.org/mozilla-central/rev/9db7cf4cc385#l13.44 the type
system always excludes calls with character pointers and so there is no need
to mention this in the comment.

The comment for the 8-bit to 16-bit AssignLiteral overload is modified a
little to use "char" instead of "character" so as not to imply that anything
other than 8-bit char parameters may be provided to that overload.

The ReplaceLiteral and InsertLiteral comments are adjusted to use "character"
instead of "char" so as not to imply that the method or comment is limited to
8-bit char parameters.

Comment 2

[Karl Tomlinson (:karlt)]

Attachment: Bug 1499212 Document static storage requirement for same-char_type AssignLiteral overload r?dbaron

Depends on D8770

Comment 3

[Karl Tomlinson (:karlt)]

Attachment: suggest non-ASCII Assign for non-static-const same-char_type array parameters

There is no advantage in AssignASCII() for char, and AssignASCII() does not
exist for char16_t array and char_type.  Similarly for ReplaceASCII and
AppendASCII.  This AssignLiteral() overload already says it is not only for
ASCII.

Depends on D8771

Comment 4

[Karl Tomlinson (:karlt)]

Attachment: document reasons for restrictions on EqualsLiteral parameter

EqualsLiteral has no opportunity to optimize for constant and static storage
duration parameters.

I don't know what would go wrong with null code point in a string, but I'm
guessing that was the reason for the explicit length contraindication.

Depends on D8773

Comment 5

[Karl Tomlinson (:karlt)]

Attachment: Bug 1499212 adjust Literal method doc so as not to imply that |this| must be a literal string r?dbaron

Depends on D8774

Comment 6

[Karl Tomlinson (:karlt)]

Attachment: Bug 1499212 align Replace/Insert/AppendLiteral parameter restrictions with those of AssignLiteral r?dbaron

There is no advantage in making these methods more restrictive on their
parameters than AssignLiteral.

The current implementation of the AppendLiteral overloads for equivalent
char_types is more permissive than AssignLiteral, but comments in the
implementation mention the possible optimization used in AssignLiteral and so
are assuming a similar constant and static storage duration restriction on its
parameter.  The optimization may never be implemented, but clients that would
benefit from support for non-constant or non-static parameters are also
expected to be rare, so there is little value in ruling out the optimization
at this stage.

ReplaceLiteral currently uses the AssignLiteral optimization.

Depends on D8775

Comment 7

[Karl Tomlinson (:karlt)]

Attachment: document that Literal parameters must have a null terminator

A character array initialized with a list of character literals will not
necessarily have a trailing null-terminator required for AssignLiteral or
trimmed in EqualsLiteral.

Depends on D8775

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5463ea36a953
remove mention of character pointers in docs for Literal methods r=dbaron
https://hg.mozilla.org/integration/autoland/rev/386c46fc6e8d
Document static storage requirement for same-char_type AssignLiteral overload r=dbaron
https://hg.mozilla.org/integration/autoland/rev/9469c37cc124
suggest non-ASCII Assign for non-static-const same-char_type array parameters r=dbaron
https://hg.mozilla.org/integration/autoland/rev/19e29511c546
document reasons for restrictions on EqualsLiteral parameter r=dbaron
https://hg.mozilla.org/integration/autoland/rev/9cc9c8d06722
adjust Literal method doc so as not to imply that |this| must be a literal string r=dbaron
https://hg.mozilla.org/integration/autoland/rev/3f6b347621b3
document that Literal parameters must have a null terminator r=dbaron
https://hg.mozilla.org/integration/autoland/rev/1728cf8a0da4
align Replace/Insert/AppendLiteral parameter restrictions with those of AssignLiteral r=dbaron

Comment 9

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/5463ea36a953
https://hg.mozilla.org/mozilla-central/rev/386c46fc6e8d
https://hg.mozilla.org/mozilla-central/rev/9469c37cc124
https://hg.mozilla.org/mozilla-central/rev/19e29511c546
https://hg.mozilla.org/mozilla-central/rev/9cc9c8d06722
https://hg.mozilla.org/mozilla-central/rev/3f6b347621b3
https://hg.mozilla.org/mozilla-central/rev/1728cf8a0da4