Bug 113200 - std::char_traits<char>::move is not constexpr when the argument is a string literal
Summary: std::char_traits<char>::move is not constexpr when the argument is a string l...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: libstdc++ (show other bugs)
Version: 13.2.0
: P3 normal
Target Milestone: 12.4
Assignee: Not yet assigned to anyone
URL:
Keywords: rejects-valid
Depends on:
Blocks: constexpr
  Show dependency treegraph
 
Reported: 2024-01-02 15:41 UTC by Peter Dimov
Modified: 2024-01-11 23:17 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2024-01-03 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Peter Dimov 2024-01-02 15:41:44 UTC
```
#include <string>
#include <cassert>

template<std::size_t N> struct S
{
    char data_[ N ];

    using traits_type = std::char_traits<char>;

    constexpr S( char const* p ): data_{}
    {
        std::size_t n = traits_type::length( p );

        assert( n < N );

        traits_type::move( data_, p, n + 1 );
    }
};

template<std::size_t N> S( char const(&)[N] ) -> S<N>;

constexpr S s( "test" );
```
(https://godbolt.org/z/PofY8MP6G)

fails with

```
In file included from /opt/compiler-explorer/gcc-trunk-20240102/include/c++/14.0.0/string:42,
                 from <source>:1:
<source>:22:23:   in 'constexpr' expansion of 'S<5>(((const char*)"test"))'
<source>:16:26:   in 'constexpr' expansion of 'std::char_traits<char>::move(((char*)(&((S<5>*)this)->S<5>::data_)), p, (n + 1))'
/opt/compiler-explorer/gcc-trunk-20240102/include/c++/14.0.0/bits/char_traits.h:423:50:   in 'constexpr' expansion of '__gnu_cxx::char_traits<char>::move(__s1, __s2, __n)'
/opt/compiler-explorer/gcc-trunk-20240102/include/c++/14.0.0/bits/char_traits.h:230:20: error: '(((const __gnu_cxx::char_traits<char>::char_type*)(& s.S<5>::data_)) == ((const char*)"test"))' is not a constant expression
  230 |           if (__s1 == __s2) // unlikely, but saves a lot of work
      |               ~~~~~^~~~~~~
```

(Reduced from a similar failure in Boost.StaticString.)
Comment 1 Andrew Pinski 2024-01-02 16:45:26 UTC
Looks more like a front-end issue ...
Comment 2 Andrew Pinski 2024-01-02 16:47:43 UTC
I take that back, clang provideds a better error message on why `&a == "string"` is not constexpr:

```
/opt/compiler-explorer/gcc-snapshot/lib/gcc/x86_64-linux-gnu/14.0.0/../../../../include/c++/14.0.0/bits/char_traits.h:230:13: note: comparison of addresses of literals has unspecified value
  230 |           if (__s1 == __s2) // unlikely, but saves a lot of work
      |                    ^
```
Comment 3 Peter Dimov 2024-01-02 16:49:15 UTC
I think that the compiler is correct; string literal address comparisons aren't constant expressions. Clang gives the same error: https://godbolt.org/z/xPWEf4z63.
Comment 4 Peter Dimov 2024-01-02 16:51:27 UTC
I didn't notice your subsequent comment, sorry. :-)
Comment 5 Jonathan Wakely 2024-01-03 11:13:45 UTC
Let's recategorize this as a front-end diagnostic issue. GCC's error is bad. That's mostly because of the ((char*)(&((S<5>*)this)->S<5>::data_)) noise, but it would also be good to explicitly state that comparisons with literals give unspecified results and so are not constant expressions.
Comment 6 Jonathan Wakely 2024-01-03 11:23:42 UTC
Although we should fix the libstdc++ problem first, then re-assign (or maybe file a separate bug for the FE diagnostic).
Comment 7 Jonathan Wakely 2024-01-03 12:57:19 UTC
Why does GCC accept this reduced version, which is invalid for the same reason as the original?

#include <string>

constexpr int N = 5;
struct S
{
    char data_[ N ];

    constexpr S( char const* p )
    {
        std::char_traits<char>::move( data_, p, N );
    }
};

constexpr S s( "test" );


Clang rejects it the same way.
Comment 8 Jonathan Wakely 2024-01-03 13:01:07 UTC
(In reply to Jonathan Wakely from comment #7)
> Why does GCC accept this reduced version, which is invalid for the same
> reason as the original?

Looks like PR 70248
Comment 9 Jiang An 2024-01-04 01:58:50 UTC
(In reply to Peter Dimov from comment #3)
> I think that the compiler is correct; string literal address comparisons
> aren't constant expressions. Clang gives the same error:
> https://godbolt.org/z/xPWEf4z63.

This looks weird... It seems that `+"abc" == +"def"` is never unspecified (must be false, but Clang rejects it in constant evaluation), while `"abcd" + 1 == +"bcd"` is unspecified.

It's unclear to me whether we can practically detect all kinds of unspecifiedness in pointer comparision involving string literals.
Comment 10 GCC Commits 2024-01-05 10:23:59 UTC
The master branch has been updated by Jonathan Wakely <redi@gcc.gnu.org>:

https://gcc.gnu.org/g:15cc291887dc9dd92b2c93f4545e20eb6c190122

commit r14-6944-g15cc291887dc9dd92b2c93f4545e20eb6c190122
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Wed Jan 3 15:01:09 2024 +0000

    libstdc++: Fix std::char_traits<C>::move [PR113200]
    
    The current constexpr implementation of std::char_traits<C>::move relies
    on being able to compare the pointer parameters, which is not allowed
    for unrelated pointers. We can use __builtin_constant_p to determine
    whether it's safe to compare the pointers directly. If not, then we know
    the ranges must be disjoint and so we can use char_traits<C>::copy to
    copy forwards from the first character to the last. If the pointers can
    be compared directly, then we can simplify the condition for copying
    backwards to just two pointer comparisons.
    
    libstdc++-v3/ChangeLog:
    
            PR libstdc++/113200
            * include/bits/char_traits.h (__gnu_cxx::char_traits::move): Use
            __builtin_constant_p to check for unrelated pointers that cannot
            be compared during constant evaluation.
            * testsuite/21_strings/char_traits/requirements/113200.cc: New
            test.
Comment 11 GCC Commits 2024-01-06 13:40:02 UTC
The releases/gcc-13 branch has been updated by Jonathan Wakely <redi@gcc.gnu.org>:

https://gcc.gnu.org/g:22601c1c25652c71c8bab4707866c020d6dad79a

commit r13-8193-g22601c1c25652c71c8bab4707866c020d6dad79a
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Wed Jan 3 15:01:09 2024 +0000

    libstdc++: Fix std::char_traits<C>::move [PR113200]
    
    The current constexpr implementation of std::char_traits<C>::move relies
    on being able to compare the pointer parameters, which is not allowed
    for unrelated pointers. We can use __builtin_constant_p to determine
    whether it's safe to compare the pointers directly. If not, then we know
    the ranges must be disjoint and so we can use char_traits<C>::copy to
    copy forwards from the first character to the last. If the pointers can
    be compared directly, then we can simplify the condition for copying
    backwards to just two pointer comparisons.
    
    libstdc++-v3/ChangeLog:
    
            PR libstdc++/113200
            * include/bits/char_traits.h (__gnu_cxx::char_traits::move): Use
            __builtin_constant_p to check for unrelated pointers that cannot
            be compared during constant evaluation.
            * testsuite/21_strings/char_traits/requirements/113200.cc: New
            test.
    
    (cherry picked from commit 15cc291887dc9dd92b2c93f4545e20eb6c190122)
Comment 12 GCC Commits 2024-01-11 23:16:24 UTC
The releases/gcc-12 branch has been updated by Jonathan Wakely <redi@gcc.gnu.org>:

https://gcc.gnu.org/g:26a9e8cee4d20e5b08c0336439c8f69a2f06af1c

commit r12-10090-g26a9e8cee4d20e5b08c0336439c8f69a2f06af1c
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Wed Jan 3 15:01:09 2024 +0000

    libstdc++: Fix std::char_traits<C>::move [PR113200]
    
    The current constexpr implementation of std::char_traits<C>::move relies
    on being able to compare the pointer parameters, which is not allowed
    for unrelated pointers. We can use __builtin_constant_p to determine
    whether it's safe to compare the pointers directly. If not, then we know
    the ranges must be disjoint and so we can use char_traits<C>::copy to
    copy forwards from the first character to the last. If the pointers can
    be compared directly, then we can simplify the condition for copying
    backwards to just two pointer comparisons.
    
    libstdc++-v3/ChangeLog:
    
            PR libstdc++/113200
            * include/bits/char_traits.h (__gnu_cxx::char_traits::move): Use
            __builtin_constant_p to check for unrelated pointers that cannot
            be compared during constant evaluation.
            * testsuite/21_strings/char_traits/requirements/113200.cc: New
            test.
    
    (cherry picked from commit 15cc291887dc9dd92b2c93f4545e20eb6c190122)
Comment 13 Jonathan Wakely 2024-01-11 23:17:12 UTC
Fixed for 12.4 and 13.3, thanks for the report.