Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[libc++][test] nasty_char_traits::move is incompatible with constexpr #74221

Closed
StephanTLavavej opened this issue Dec 3, 2023 · 0 comments · Fixed by #90981
Closed

[libc++][test] nasty_char_traits::move is incompatible with constexpr #74221

StephanTLavavej opened this issue Dec 3, 2023 · 0 comments · Fixed by #90981
Assignees
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. test-suite

Comments

@StephanTLavavej
Copy link
Member

Found while running libc++'s test suite with MSVC's STL.

nasty_char_traits::move is marked constexpr but compares unrelated pointers s1 < s2. This is forbidden, and nasty_char_traits::copy acknowledges this immediately below:

constexpr nasty_char* nasty_char_traits::move(nasty_char* s1, const nasty_char* s2, std::size_t n) {
nasty_char* r = s1;
if (s1 < s2) {
for (; n; --n, ++s1, ++s2)
assign(*s1, *s2);
} else if (s2 < s1) {
s1 += n;
s2 += n;
for (; n; --n)
assign(*--s1, *--s2);
}
return r;
}
constexpr nasty_char* nasty_char_traits::copy(nasty_char* s1, const nasty_char* s2, std::size_t n) {
if (!std::is_constant_evaluated()) // fails in constexpr because we might be comparing unrelated pointers
assert(s2 < s1 || s2 >= s1 + n);
nasty_char* r = s1;
for (; n; --n, ++s1, ++s2)
assign(*s1, *s2);
return r;
}

Click to expand compiler error:

With libc++'s test suite, MSVC's STL, and Clang/LLVM, std/strings/basic.string/string.modifiers/string_append/initializer_list.pass.cpp emits this error:

D:\GitHub\STL\llvm-project\libcxx\test\std\strings\basic.string\string.modifiers\string_append\initializer_list.pass.cpp(53,17): error: static assertion expression is not an integral constant expression
  static_assert(test());
                ^~~~~~
D:\GitHub\STL\llvm-project\libcxx\test\support\nasty_string.h(122,10): note: comparison between '&s._Mypair._Myval2._Bx._Buf[3]' and '&{CharT(('a')), CharT(('b')), CharT(('c'))}[0]' has unspecified value
  if (s1 < s2) {
         ^
D:\GitHub\STL\out\x64\out\inc\xstring(3311,13): note: in call to 'move(&s._Mypair._Myval2._Bx._Buf[3], &{CharT(('a')), CharT(('b')), CharT(('c'))}[0], 3)'
            _Traits::move(_Old_ptr + _Old_size, _Ptr, _Count);
            ^
D:\GitHub\STL\out\x64\out\inc\xstring(3152,16): note: in call to '&s->append(&{CharT(('a')), CharT(('b')), CharT(('c'))}[0], 3)'
        return append(_Ilist.begin(), _Convert_size<size_type>(_Ilist.size()));
               ^
D:\GitHub\STL\llvm-project\libcxx\test\std\strings\basic.string\string.modifiers\string_append\initializer_list.pass.cpp(27,5): note: in call to '&s->append({&{CharT(('a')), CharT(('b')), CharT(('c'))}[0], &{CharT(('a')), CharT(('b')), CharT(('c'))}[3]})'
  s.append({CharT('a'), CharT('b'), CharT('c')});
    ^
D:\GitHub\STL\llvm-project\libcxx\test\std\strings\basic.string\string.modifiers\string_append\initializer_list.pass.cpp(44,3): note: in call to 'test()'
  test<nasty_string>();
  ^
D:\GitHub\STL\llvm-project\libcxx\test\std\strings\basic.string\string.modifiers\string_append\initializer_list.pass.cpp(53,17): note: in call to 'test()'
  static_assert(test());
                ^

In microsoft/STL's product code, I have a truly marvelous way to avoid this problem - a linear scan to detect whether the first iterator of the destination is within the source range, in which case a backward loop is necessary. See https://github.com/microsoft/STL/blob/0403d19f5461fd15983737c3f01ec34800ea9275/stl/inc/xstring#L85-L93 .

@github-actions github-actions bot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Dec 3, 2023
@mordante mordante self-assigned this May 2, 2024
mordante added a commit to mordante/llvm-project that referenced this issue May 3, 2024
This was discovered by @StephanTLavavej who provided the solution they use
in MSVC STL. This solution is based on that example.

The same issue affects the constexpr_char_traits which was discovered in
llvm#88389. This uses the same fix.

Fixes: llvm#74221
mordante added a commit to mordante/llvm-project that referenced this issue May 8, 2024
This was discovered by @StephanTLavavej who provided the solution they use
in MSVC STL. This solution is based on that example.

The same issue affects the constexpr_char_traits which was discovered in
llvm#88389. This uses the same fix.

Fixes: llvm#74221
mordante added a commit that referenced this issue May 9, 2024
The issue in nasty_char_traits was discovered by @StephanTLavavej who
provided
the solution they use in MSVC STL. This solution is based on that
example.

The same issue affects the constexpr_char_traits which was discovered in
#88389. This uses the same fix.

Fixes: #74221
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. test-suite
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants