[llvm-bugs] [Bug 51018] New: C++23 makes constructing StringRef from SmallString ambiguous

[via llvm-bugs]

https://bugs.llvm.org/show_bug.cgi?id=51018

            Bug ID: 51018
           Summary: C++23 makes constructing StringRef from SmallString
                    ambiguous
           Product: new-bugs
           Version: trunk
          Hardware: PC
                OS: Windows NT
            Status: NEW
          Severity: normal
          Priority: P
         Component: new bugs
          Assignee: unassignedbugs at nondot.org
          Reporter: sfinae at hotmail.com
                CC: htmldeveloper at gmail.com, llvm-bugs at lists.llvm.org

I work on MSVC's C++ Standard Library implementation, and we regularly build
LLVM (along with many other open-source projects) to identify compiler/library
bugs that would break your project, so we can fix them before shipping. This
also allows us to provide advance notice of source-breaking changes - which is
the case here.

The paper P1989R2 "Range Constructor For string_view" (see
https://wg21.link/P1989R2 ) has been accepted for the upcoming C++23 Standard,
and we recently implemented it by merging
https://github.com/microsoft/STL/pull/2000 from our contributor @sam20908. Our
open-source test pass then discovered the following code in LLVM that is now
disallowed by C++23:

https://github.com/llvm/llvm-project/blob/74c308c56a2d0f000dfed3287311ce46a94ae3c8/llvm/lib/Support/CommandLine.cpp#L867
https://github.com/llvm/llvm-project/blob/74c308c56a2d0f000dfed3287311ce46a94ae3c8/llvm/lib/Support/CommandLine.cpp#L881
NewArgv.push_back(Saver.save(StringRef(Token)).data());

Where Token is defined as:

https://github.com/llvm/llvm-project/blob/74c308c56a2d0f000dfed3287311ce46a94ae3c8/llvm/lib/Support/CommandLine.cpp#L826
SmallString<128> Token;

With our updated STL in C++23 mode, MSVC rejects this code with:

error C2440: '<function-style-cast>': cannot convert from
'llvm::SmallString<128>' to 'llvm::StringRef'
note: No constructor could take the source type, or constructor overload
resolution was ambiguous

SmallString has an implicit conversion operator to StringRef, which appears to
be desired:

https://github.com/llvm/llvm-project/blob/74c308c56a2d0f000dfed3287311ce46a94ae3c8/llvm/include/llvm/ADT/SmallString.h#L269
operator StringRef() const { return str(); }

However, StringRef is implicitly constructible from std::string_view:

https://github.com/llvm/llvm-project/blob/74c308c56a2d0f000dfed3287311ce46a94ae3c8/llvm/include/llvm/ADT/StringRef.h#L120
/*implicit*/ constexpr StringRef(std::string_view Str)

Previously (in C++20 and below) this didn't matter. Now, C++23 has added a
range constructor to std::string_view, and SmallString provides begin() and
end() via SmallVector => SmallVectorImpl => SmallVectorTemplateBase =>
SmallVectorTemplateCommon (I think):

https://github.com/llvm/llvm-project/blob/74c308c56a2d0f000dfed3287311ce46a94ae3c8/llvm/include/llvm/ADT/SmallVector.h#L253-L256

Because the usage is StringRef(Token), the rule "can't perform two implicit
user-defined conversions in a row" doesn't apply - this is attempting to
explicitly construct a StringRef, so it's willing to consider SmallString's
implicit `operator StringRef()`, and it's willing to consider
basic_string_view's implicit constructor basic_string_view(R&&) (binding to the
SmallString Token) followed by constructing StringRef from std::string_view
(the usage is explicit, even though the constructor is implicit). These are
ambiguous. Thanks to Tim Song for analyzing what happened here.

My apologies for not providing a self-contained repro here - I hope that this
information should be sufficient, and you should see this with libc++ as soon
as it implements P1989R2.

There are many ways to fix this code in a backwards-compatible manner, but I
suspect that the simplest is to use SmallString's named member function str():

https://github.com/llvm/llvm-project/blob/74c308c56a2d0f000dfed3287311ce46a94ae3c8/llvm/include/llvm/ADT/SmallString.h#L258-L259
/// Explicit conversion to StringRef.
StringRef str() const { return StringRef(this->data(), this->size()); }

Thus changing llvm/lib/Support/CommandLine.cpp lines 867 and 881 from:

NewArgv.push_back(Saver.save(StringRef(Token)).data());

to:

NewArgv.push_back(Saver.save(Token.str()).data());

-- 
You are receiving this mail because:
You are on the CC list for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-bugs/attachments/20210708/ea9aa076/attachment.html>