[PATCH] D53509: Add unsgined char StringRef constructor/Fix llvm-strings crash

Duncan P. N. Exon Smith via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Oct 22 11:08:57 PDT 2018


dexonsmith added inline comments.


================
Comment at: include/llvm/ADT/StringRef.h:93-97
+    /// Construct a string ref from a pointer to bytes and length.
+    LLVM_ATTRIBUTE_ALWAYS_INLINE
+    /*implicit*/ constexpr StringRef(const unsigned char *data, size_t length)
+        : Data(reinterpret_cast<const char *>(data)), Length(length) {}
+
----------------
ruiu wrote:
> Not sure if this is a good thing to do. If you do, you probably should do the same for `StringRef::StringRef(const char *Str)` and other functions? But isn't that a bit too much?
> If you do, you probably should do the same for `StringRef::StringRef(const char *Str)` and other functions?

IMO `StringRef::StringRef(const char *Str)` should only be used for string literals.  Same with `StringRef::withNullAsEmpty()`.


Repository:
  rL LLVM

https://reviews.llvm.org/D53509





More information about the llvm-commits mailing list