Bug 105995 - QoI: constexpr basic_string variable must use all of its SSO buffer
Summary: QoI: constexpr basic_string variable must use all of its SSO buffer
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: libstdc++ (show other bugs)
Version: 12.1.0
: P3 enhancement
Target Milestone: 12.2
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-06-16 07:13 UTC by Jiang An
Modified: 2022-08-03 13:49 UTC (History)
0 users

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2022-06-16 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Jiang An 2022-06-16 07:13:57 UTC
The current implementation in libstdc++ only supports constexpr std::basic_string variable of static storage duration which uses all of its SSO buffer:

#include <string>

constexpr std::string str15{"0123456789abcde"};
// constexpr std::string str14{"0123456789abcd"}; // error: should we support this?
// constexpr std::string str16{"0123456789abcdef"}; // error: unsupportable

constexpr std::u16string u16str7{u"0123456"};
constexpr std::u32string u32str3{U"012"};

int main()
{
    // constexpr std::string strdyn{"0123456789abcde"}; // error: unsupportable
}

Compiler Explorer links:
https://gcc.godbolt.org/z/cvbY1hK3G
https://gcc.godbolt.org/z/634888Y96

I think constexpr string variable that requires heap allocation is currently unimplementable, and constexpr string variable of dynamic storage duration can't be supported by libstdc++-v3 without ABI breakage now.

However, should we allow shorter strings to be stored into constexpr basic_string variables? We can do this by always fully initializing the SSO buffer (if it's used) during constant evluation.
Comment 1 Andrew Pinski 2022-06-16 07:19:50 UTC
C++20 supports dynamic allocation for constexpr.
Comment 2 Jiang An 2022-06-16 07:41:43 UTC
(In reply to Andrew Pinski from comment #1)
> C++20 supports dynamic allocation for constexpr.

Yeah, but a constexpr variable can't hold dynamically allocated memory. Dynamically allocated memory must be deallocated within its initialization.
Comment 3 Jonathan Wakely 2022-06-16 11:08:43 UTC
Right, you can only use std::string as a local variable in a constexpr function. I don't think it's expected to work as a constexpr variable.
Comment 4 Jonathan Wakely 2022-06-16 11:14:19 UTC
This makes it work:

--- a/libstdc++-v3/include/bits/basic_string.h
+++ b/libstdc++-v3/include/bits/basic_string.h
@@ -352,7 +352,8 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
       {
 #if __cpp_lib_is_constant_evaluated
        if (std::is_constant_evaluated())
-         _M_local_buf[0] = _CharT();
+         for (_CharT& __c : _M_local_buf)
+           __c = _CharT();
 #endif
        return _M_local_data();
       }
Comment 5 GCC Commits 2022-06-16 19:20:53 UTC
The master branch has been updated by Jonathan Wakely <redi@gcc.gnu.org>:

https://gcc.gnu.org/g:98a0d72a610a87e8e383d366e50253ddcc9a51dd

commit r13-1139-g98a0d72a610a87e8e383d366e50253ddcc9a51dd
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Thu Jun 16 14:57:32 2022 +0100

    libstdc++: Support constexpr global std::string for size < 15 [PR105995]
    
    I don't think this is required by the standard, but it's easy to
    support.
    
    libstdc++-v3/ChangeLog:
    
            PR libstdc++/105995
            * include/bits/basic_string.h (_M_use_local_data): Initialize
            the entire SSO buffer.
            * testsuite/21_strings/basic_string/cons/char/105995.cc: New test.
Comment 6 Jonathan Wakely 2022-06-16 19:21:46 UTC
Fixed for GCC 13, thanks for the report.
Comment 7 GCC Commits 2022-08-03 13:47:04 UTC
The releases/gcc-12 branch has been updated by Jonathan Wakely <redi@gcc.gnu.org>:

https://gcc.gnu.org/g:e562236851e06091256593aa0d3fbda60a28e45b

commit r12-8657-ge562236851e06091256593aa0d3fbda60a28e45b
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Thu Jun 16 14:57:32 2022 +0100

    libstdc++: Support constexpr global std::string for size < 15 [PR105995]
    
    I don't think this is required by the standard, but it's easy to
    support.
    
    libstdc++-v3/ChangeLog:
    
            PR libstdc++/105995
            * include/bits/basic_string.h (_M_use_local_data): Initialize
            the entire SSO buffer.
            * testsuite/21_strings/basic_string/cons/char/105995.cc: New test.
    
    (cherry picked from commit 98a0d72a610a87e8e383d366e50253ddcc9a51dd)
Comment 8 Jonathan Wakely 2022-08-03 13:49:32 UTC
Backported for 12.2