issues with comments re "Literal" methods

RESOLVED FIXED in Firefox 64

Status

()

enhancement
P3
normal
RESOLVED FIXED
10 months ago
10 months ago

People

(Reporter: karlt, Assigned: karlt)

Tracking

Trunk
mozilla64
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox64 fixed)

Details

Attachments

(7 attachments)

+++ 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.
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.
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
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
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
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
Attachment #9017324 - Attachment description: Bug 1499212 suggest non-ASCII methods for non-static same-char_type array assign/replace/append r?dbaron → suggest non-ASCII Assign for non-static-const same-char_type array parameters
Attachment #9017325 - Attachment description: Bug 1499212 remove requirement that EqualsLiteral parameter is constant r?dbaron → document reasons for restrictions on EqualsLiteral parameter
Attachment #9017775 - Attachment description: Bug 1499212 document that Literal parameters must have string literal initializers r?dbaron → document that Literal parameters must have a null terminator
Pushed by ktomlinson@mozilla.com:
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
You need to log in before you can comment on or make changes to this bug.