Open Bug 1602452 Opened 1 year ago Updated 1 month ago

Several nsTLiteralString/nsTStringRepr member functions should be constexpr

Categories

(Core :: XPCOM, task)

task
Not set
normal

Tracking

()

People

(Reporter: sg, Assigned: sg)

Details

(Keywords: leave-open)

Attachments

(2 files, 1 obsolete file)

No description provided.

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.

Pushed by sgiesecke@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/bf906a905a26
Make member functions of nsTStringRepr defined in header constexpr. r=froydnj
Keywords: leave-open

(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

Flags: needinfo?(sgiesecke)
Pushed by sgiesecke@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/2c6f4188a5bc
Make member functions of nsTStringRepr defined in header constexpr. r=froydnj
Attachment #9114829 - Attachment description: Bug 1602452 - Make most remaining nsTStringRepr functions constexpr. → Bug 1602452 - Make most remaining nsTStringRepr functions constexpr. r=froydnj
Attachment #9115096 - Attachment is obsolete: true

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?

Flags: needinfo?(nfroyd)
Status: ASSIGNED → NEW

(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. :)

Flags: needinfo?(nfroyd)
Component: String → XPCOM
You need to log in before you can comment on or make changes to this bug.