Several nsTLiteralString/nsTStringRepr member functions should be constexpr
Categories
(Core :: XPCOM, task)
Tracking
()
People
(Reporter: sg, Unassigned)
Details
Attachments
(2 files, 1 obsolete file)
Reporter | ||
Comment 1•5 years ago
|
||
This is easy for the member functions defined in the header, it is just necessary to add constexpr
.
It is also interesting for Equals
and Compare
, but the definitions of those functions need to be moved to the header as well. They could be implemented based on std::string_view, which already provides constexpr
comparison operators.
Reporter | ||
Comment 2•5 years ago
|
||
Comment 4•5 years ago
|
||
Backed out changeset bf906a905a26 (Bug 1602452) for causing bustages in gtest/TestStrings.cpp
Backout link: https://hg.mozilla.org/integration/autoland/rev/0df1383a748325b5957d1dc5427eb772d434f4f8
Failure logs:
Reporter | ||
Updated•5 years ago
|
Reporter | ||
Comment 5•5 years ago
|
||
(In reply to Alexandru Michis [:malexandru] from comment #4)
Backed out changeset bf906a905a26 (Bug 1602452) for causing bustages in gtest/TestStrings.cpp
Windows build failures should be fixed by updated patch, running on try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ca71755198a7fa3bafe9b61c5aff4efecb74a1ec
Reporter | ||
Comment 7•5 years ago
|
||
Updated•5 years ago
|
Reporter | ||
Comment 8•5 years ago
|
||
Depends on D56682
Comment 9•5 years ago
|
||
bugherder |
Updated•5 years ago
|
Reporter | ||
Comment 10•5 years ago
|
||
After abandoning revision https://bugzilla.mozilla.org/attachment.cgi?id=9115096:
My original idea was to allow to write something like:
constexpr auto a = NS_LITERAL_STRING("a");
constexpr auto b = NS_LITERAL_STRING("b");
constexpr auto ab = a + b;
This would frequently be helpful. However, this is not possible since nsTSubstringTuple is a MOZ_TEMPORARY_CLASS. Alternatively, another overload of operator+ accepting only nsTLiteralString (or perhaps more explicitly a named function, e.g. ConcatConstexpr(...)) that allows to do this, could be defined, which doesn't return nsTSubstringTuple but a new class, which does not need to be MOZ_TEMPORARY_CLASS and might be based on an array rather than a binary tree. (I have done this in the past in another codebase.) What do you think about that?
Reporter | ||
Updated•5 years ago
|
Reporter | ||
Updated•5 years ago
|
Comment 11•4 years ago
|
||
(In reply to Simon Giesecke [:sg] [he/him] from comment #10)
After abandoning revision https://bugzilla.mozilla.org/attachment.cgi?id=9115096:
My original idea was to allow to write something like:
constexpr auto a = NS_LITERAL_STRING("a"); constexpr auto b = NS_LITERAL_STRING("b"); constexpr auto ab = a + b;
This would frequently be helpful. However, this is not possible since nsTSubstringTuple is a MOZ_TEMPORARY_CLASS. Alternatively, another overload of operator+ accepting only nsTLiteralString (or perhaps more explicitly a named function, e.g. ConcatConstexpr(...)) that allows to do this, could be defined, which doesn't return nsTSubstringTuple but a new class, which does not need to be MOZ_TEMPORARY_CLASS and might be based on an array rather than a binary tree. (I have done this in the past in another codebase.) What do you think about that?
I am ambivalent on this, but it's not really my decision anymore. :)
Assignee | ||
Updated•4 years ago
|
Reporter | ||
Comment 12•4 years ago
|
||
Nika, I think we discussed the question from Comment 10 some time ago and I think you also said this wouldn't be widely useful (the parts of the codebase that use SQL queries like dom/indexedDB might be special here). The question remains if at least some of the constexpr'ing changes of the attached patch (https://phabricator.services.mozilla.com/D56525) are still useful.
Comment 13•4 years ago
|
||
I don't think they're necessary right now, so we can probably hold off on the patch for now. Thanks
Reporter | ||
Updated•4 years ago
|
Updated•4 years ago
|
Reporter | ||
Updated•4 years ago
|
Comment 14•3 years ago
|
||
The leave-open keyword is there and there is no activity for 6 months.
:nika, maybe it's time to close this bug?
Comment 15•3 years ago
|
||
Closing as I don't think we intend to fix this in the near-term.
Updated•3 years ago
|
Description
•