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

[Duncan P. N. Exon Smith via Phabricator via llvm-commits]

dexonsmith added a comment.

In D103935#2879061 <https://reviews.llvm.org/D103935#2879061>, @tstellar wrote:

> This patch causes segmentation faults when you have an application compiled with -std=c++17 (The current gcc default) and libLLVM.so compiled with c++14 (The current LLVM project default).  Is this a valid use case or do we require that all library users use the same c++ version as the library.

Seems like valid/important use case to me!

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



================
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!");
+    }
----------------
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?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103935