[PATCH] D106186: Avoid keeping internal string_views in Twine.

[David Blaikie via Phabricator via llvm-commits]

dblaikie accepted this revision.
dblaikie added a comment.
This revision is now accepted and ready to land.

Looks good to me, with the (ptr, length) ctor made private.

@dexonsmith could you double check this & see if it sounds reasonable?



================
Comment at: llvm/include/llvm/ADT/Twine.h:298-303
+    /*implicit*/ Twine(const char *ptr, size_t length)
+        : LHSKind(PtrAndLengthKind) {
+      LHS.ptrAndLength.ptr = ptr;
+      LHS.ptrAndLength.length = length;
       assert(isValid() && "Invalid twine!");
     }
----------------
I probably wouldn't add this ctor (if it's being added it should probably be unit tested - which I guess would be a perk of this design, that most of the string_view implementation would be unit testable even when string_view isn't available) - but if it's staging for common code, it could be added, but made private - then when other things (like StringRef) is changed over to use this ctor it'll get that generic (non-C++17-specific) test coverage.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106186