<html>
    <head>
      <base href="https://bugs.llvm.org/">
    </head>
    <body><table border="1" cellspacing="0" cellpadding="8">
        <tr>
          <th>Bug ID</th>
          <td><a class="bz_bug_link 
          bz_status_NEW "
   title="NEW - C++23 makes constructing StringRef from SmallString ambiguous"
   href="https://bugs.llvm.org/show_bug.cgi?id=51018">51018</a>
          </td>
        </tr>

        <tr>
          <th>Summary</th>
          <td>C++23 makes constructing StringRef from SmallString ambiguous
          </td>
        </tr>

        <tr>
          <th>Product</th>
          <td>new-bugs
          </td>
        </tr>

        <tr>
          <th>Version</th>
          <td>trunk
          </td>
        </tr>

        <tr>
          <th>Hardware</th>
          <td>PC
          </td>
        </tr>

        <tr>
          <th>OS</th>
          <td>Windows NT
          </td>
        </tr>

        <tr>
          <th>Status</th>
          <td>NEW
          </td>
        </tr>

        <tr>
          <th>Severity</th>
          <td>normal
          </td>
        </tr>

        <tr>
          <th>Priority</th>
          <td>P
          </td>
        </tr>

        <tr>
          <th>Component</th>
          <td>new bugs
          </td>
        </tr>

        <tr>
          <th>Assignee</th>
          <td>unassignedbugs@nondot.org
          </td>
        </tr>

        <tr>
          <th>Reporter</th>
          <td>sfinae@hotmail.com
          </td>
        </tr>

        <tr>
          <th>CC</th>
          <td>htmldeveloper@gmail.com, llvm-bugs@lists.llvm.org
          </td>
        </tr></table>
      <p>
        <div>
        <pre>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
<a href="https://wg21.link/P1989R2">https://wg21.link/P1989R2</a> ) has been accepted for the upcoming C++23 Standard,
and we recently implemented it by merging
<a href="https://github.com/microsoft/STL/pull/2000">https://github.com/microsoft/STL/pull/2000</a> from our contributor @sam20908. Our
open-source test pass then discovered the following code in LLVM that is now
disallowed by C++23:

<a href="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#L867</a>
<a href="https://github.com/llvm/llvm-project/blob/74c308c56a2d0f000dfed3287311ce46a94ae3c8/llvm/lib/Support/CommandLine.cpp#L881">https://github.com/llvm/llvm-project/blob/74c308c56a2d0f000dfed3287311ce46a94ae3c8/llvm/lib/Support/CommandLine.cpp#L881</a>
NewArgv.push_back(Saver.save(StringRef(Token)).data());

Where Token is defined as:

<a href="https://github.com/llvm/llvm-project/blob/74c308c56a2d0f000dfed3287311ce46a94ae3c8/llvm/lib/Support/CommandLine.cpp#L826">https://github.com/llvm/llvm-project/blob/74c308c56a2d0f000dfed3287311ce46a94ae3c8/llvm/lib/Support/CommandLine.cpp#L826</a>
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:

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

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

<a href="https://github.com/llvm/llvm-project/blob/74c308c56a2d0f000dfed3287311ce46a94ae3c8/llvm/include/llvm/ADT/StringRef.h#L120">https://github.com/llvm/llvm-project/blob/74c308c56a2d0f000dfed3287311ce46a94ae3c8/llvm/include/llvm/ADT/StringRef.h#L120</a>
/*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):

<a href="https://github.com/llvm/llvm-project/blob/74c308c56a2d0f000dfed3287311ce46a94ae3c8/llvm/include/llvm/ADT/SmallVector.h#L253-L256">https://github.com/llvm/llvm-project/blob/74c308c56a2d0f000dfed3287311ce46a94ae3c8/llvm/include/llvm/ADT/SmallVector.h#L253-L256</a>

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():

<a href="https://github.com/llvm/llvm-project/blob/74c308c56a2d0f000dfed3287311ce46a94ae3c8/llvm/include/llvm/ADT/SmallString.h#L258-L259">https://github.com/llvm/llvm-project/blob/74c308c56a2d0f000dfed3287311ce46a94ae3c8/llvm/include/llvm/ADT/SmallString.h#L258-L259</a>
/// 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());</pre>
        </div>
      </p>


      <hr>
      <span>You are receiving this mail because:</span>

      <ul>
          <li>You are on the CC list for the bug.</li>
      </ul>
    </body>
</html>