[PATCH] D103935: Add Twine support for std::string_view.

[Sterling Augustine via Phabricator via llvm-commits]

saugustine added a comment.

In D103935#2881772 <https://reviews.llvm.org/D103935#2881772>, @dexonsmith wrote:

> Seems like valid/important use case to me!

The ODR violation issues would need to be resolved to support this case.

> I wonder if the Twine part of the patch should wait for LLVM to drop support for C++14.

This patch follows on another one that enables Twine support everywhere over std::string. Before that, because strings are trivially convertable to string_views, there was no ambiguity. With that change there is ambiguity in many places, and causes similar problems to the present one for downstream C++ 17 users. Not sure which case should win.



================
Comment at: llvm/include/llvm/ADT/Twine.h:299-303
+    /*implicit*/ Twine(const std::string_view &Str)
+        : LHSKind(StdStringViewKind) {
+      LHS.stdStringView = &Str;
+      assert(isValid() && "Invalid twine!");
+    }
----------------
dexonsmith wrote:
> Is this constructor necessary? I would have expected this to get picked up as a conversion by the `StringRef` constructor... but maybe there are two candidates?
There are indeed two candidates, so we need this one to disambiguate.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103935/new/

https://reviews.llvm.org/D103935