Open Bug 1593666 Opened 5 years ago Updated 2 years ago

Specify preferred way to construct literal empty and non-empty strings

Categories

(Core :: DOM: File, task, P3)

task

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.

Agreed on:

Thanks for your continued efforts and leadership here!

I opened bug 1594389 for modifying/amending Empty[C]String.

Priority: -- → P3
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.