Bug 158068 - Replace string literals with custom O(U)String literals in code
Summary: Replace string literals with custom O(U)String literals in code
Status: NEW
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: LibreOffice (show other bugs)
Version:
(earliest affected)
unspecified
Hardware: All All
: medium normal
Assignee: Not Assigned
URL:
Whiteboard: target:24.2.0 target:25.2.0 target:25...
Keywords: difficultyBeginner, easyHack, skillCpp, topicCleanup
Depends on:
Blocks: Dev-related
  Show dependency treegraph
 
Reported: 2023-11-05 08:01 UTC by Mike Kaganski
Modified: 2025-03-28 07:04 UTC (History)
3 users (show)

See Also:
Crash report or crash signature:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Mike Kaganski 2023-11-05 08:01:01 UTC
Throughout the codebase, there are hundreds of initializations of O(U)Strings with string literals, like

    OUString foo = "abc";
    OString bar("def");
    std::vector<OUString> baz = {"xyz1", "xyz2", "xyz3"};

Every time such an initialization appears, a string constructor is called at runtime, which allocates memory, and copies strings. This is because O(U)String is not a plain character array, but a class with a pointer to a structure rtl_(u)String that holds information about reference count, size, and the actual character array.

To avoid overhead of such construction, several techniques were introduced over time; with C++17 adoption, we used an updated O(U)StringLiteral, which is a templated structure, with a layout compatible with rtl_(u)String; such O(U)StringLiterals are created at compile time (typically as static inline constexpr objects), and creation of OUStrings from those became a trivial operation. But that required a bloat of such helper objects, which is inconvenient. Instead of a clear

    for (;;)
    {
        // ...
        OUString foo = "abc"; // Allocating memory in a loop
        // doing something with the string...
    }

we had 

    static constexpr OUStringLiteral a_abc("abc"); // compile-time constant
    // ...
    for (;;)
    {
        // ...
        OUString foo(a_abc); // Trivial construction without allocation
        // doing something with the string...
    }

This inconvenience prevented wide adoption of the optimization; only in few cases, plain "abc" used to initialize O(U)Strings at run time were replaced with uses of O(U)StringLiterals; most of run-time initializations stay in code.

Introduction of C++20 support in the codebase (commit 1eef07805021b7ca26a1a8894809b6d995747ba1 Bump baseline to C++20, 2023-09-22) pawed a way to use custom literals; and Stephan came with a solution that allows to (mostly) get rid of use of the intermediate O(U)StringLiteral objects, and have the benefir of compile-time creation of the string objects: commit 27d1f3ac016d77d3c907cebedca558308f366855 (O[U]String literals (unusable for now, C++20 only), 2023-07-14) introduced the operator ""_ostr() to both OString and OUString; and commit e83e62fe376a91f7270435e06ee7f6864c48fb4b (Work around MSVC bug with "..."_ostr vs. u"..."_ostr, 2023-07-19) renamed it in OUString into operator ""_ustr(). These operators still use O(U)StringLiterals internally, but hide it from the programmer.

Now it is possible to write

    for (;;)
    {
        // ...
        OUString foo = u"abc"_ustr; // trivial initialization using a compile-time object
        // doing something with the string...
    }

While the u"abc"_ustr or "abc"_ostr syntax is a bit bulkier than simple "abc", it is inline, is unambiguous, and provides the wanted optimization using much better developer experience than the previous O(U)StringLiteral solution.

The easy hack is to replace uses of run-time initializations of O(U)Strings using string literals like "abc" in codebase with the user-defined literals:

    OUString foo("abc");

should become

    OUString foo(u"abc"_ustr);

Note that not every string literal in code needs to be replaced: e.g., comparison like

    OUString foo;
    //...
    if (foo == "abc")

is no less efficient than comparing OUString to another OUString; also, concatenations happening at run time, that use such literals, are also very efficient, and replacing such literals there would not improve things. So e.g.

    OUString foo("abc" + OUString::number(n))

should *not* be changed to

    OUString foo(u"abc"_ustr + OUString::number(n))
Comment 1 Commit Notification 2023-11-05 18:54:56 UTC
Mike Kaganski committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/b65f295698d3c09a1d18336cb17ac29487266400

tdf#158068: an example of "abc" -> u"abc"_ustr conversion

It will be available in 24.2.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 2 Commit Notification 2024-12-03 19:47:03 UTC
anonymotter committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/7a12424df2b46385897b48c304bca867415b3f88

tdf#158068 Replace string literals with OUString literals in canvas

It will be available in 25.2.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 3 Commit Notification 2025-02-14 15:57:36 UTC
Simon Chenery committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/391e1d1a39ff9d3fd407253ef79354261fd6e7c3

tdf#158068 Use OUString literals in DataInterpreter.cxx

It will be available in 25.8.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 4 Commit Notification 2025-02-17 11:27:22 UTC
Simon Chenery committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/d58a0238561584cb5440e8df79fbf06737ae42af

tdf#158068 Use OUString literals in printerinfomanager.cxx

It will be available in 25.8.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 5 Commit Notification 2025-02-18 08:30:32 UTC
Simon Chenery committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/685899f24e58653ad274c518d84610a41880f6da

tdf#158068 Use OUString literals in convert3to4.cxx

It will be available in 25.8.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 6 Commit Notification 2025-02-18 08:34:36 UTC
Simon Chenery committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/2aba56b511894e7b8be4e6b10413b1b64f86bc3a

tdf#158068 Use OUString literals in targetlistener.cxx

It will be available in 25.8.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 7 Commit Notification 2025-02-21 07:47:09 UTC
Simon Chenery committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/2aaa35d2a2281b87b8803bab990592099100350f

tdf#158068 Use OUString literals in dbmgr.cxx

It will be available in 25.8.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 8 Commit Notification 2025-03-12 08:29:07 UTC
Bogdan Buzea committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/817cb41005db87c9bdd63663e5d1d1357d6e5cbc

tdf#158068: Replace with static constexpr O(U)String

It will be available in 25.8.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 9 Commit Notification 2025-03-16 11:28:09 UTC
Stephan Bergmann committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/26d0af9ddc98355865c62ce0ffc04c3919ebce87

Revert "tdf#158068: Replace with static constexpr O(U)String"

It will be available in 25.8.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 10 Commit Notification 2025-03-28 07:04:36 UTC
Devashish Gupta committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/fbd888bfc8e916361afbc55dbfa329ebaab6d2d3

tdf#158068: Replace OUString literal with u'...'_ustr in getOSVersion

It will be available in 25.8.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.