Specify preferred way to construct literal empty and non-empty strings
Categories
(Core :: DOM: File, task, P3)
Tracking
()
People
(Reporter: sg, Unassigned)
References
(Blocks 1 open bug)
Details
There are several different variants to instantiate an empty C-string (analogously for a 16-bit string):
NS_LITERAL_CSTRING("")
nsLiteralCString("")
EmptyCString()
nsCString()
(not entirely sure if this is equivalent, this is not well-documented)nsCString("")
nsDependentCString("")
Only the first two of these are constexpr
, and allows for exploiting their literal-ness at compile time by custom overloads of functions/operators for nsLiteral[C]String
(without needing to rely on compiler optimizations, which are never done in debug builds, and probably will also not be done in release builds given the complexity of ns[C]String
).
Since the NS_LITERAL_[C]STRING
macros exist on purpose to avoid accidental misuse of nsLiteral[C]String
with non-literals, I suggest to recommend the use of NS_LITERAL_CSTRING("")
by default.
For non-empty literal strings the benefits of NS_LITERAL_[C]STRING
are even more obvious (it is not necessary to calculate the length at run-time, and no copies of the string data are needed), and so the macros should be used ubiquitously in those cases.
In addition, NS_NAMED_LITERAL_[C]STRING(name, "foo");
should be discouraged in favor of constexpr auto foo = NS_LITERAL_[C]STRING("foo");
(reducing the non-standard syntax to the necessary minimum).
I am not sure if there are situations at all where using Empty[C]String()
would provide significant benefits over NS_LITERAL_[C]STRING("")
. Theoretically, this might happen when the conversion from a nsLiteral[C]String
to a ns[C]String
occurs which is not optimized away. However, such a conversion will not always be necessary, since there is custom handling of nsLiteral[C]String
in some contexts.
Side remark (which, if found worthwhile to pursue, should be raised as an additional bug for the String component):
Based on this, it could be beneficial to change Empty[C]String
to be constexpr and return a nsLiteral[C]String
(by value) instead of returning a const reference to a ns[C]String
. A function returning a ns[C]String
by value or reference cannot be constexpr, since nsTSubstring
has a user-provided destructor, which can probably not be eliminated.
Comment 1•5 years ago
|
||
Agreed on:
- Explicitly providing guidance here. https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM/Guide/Internal_strings is great but verbose, and NS_LITERAL_* is what https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM/Guide/Internal_strings#Literal_Strings indicates to use.
- NS_NAMED_LITERAL_* is weird and awkward and suggesting a non-awkward way is great. It might make sense to try and flow some of that guidance back into the page above, but it's not a necessity as it may result in a bit o fbike-shedding.
- Good to help optimize Empty[C]String() and provide clarity about it. I do personally like it over NS_LITERAL_STRING("") both from a readability perspective and from a "works better for auditing empty versus void strings using searchfox" perspective.
Thanks for your continued efforts and leadership here!
Reporter | ||
Comment 2•5 years ago
|
||
I opened bug 1594389 for modifying/amending Empty[C]String
.
Updated•5 years ago
|
Updated•2 years ago
|
Description
•