[PATCH] D120990: [demangler] Add StringView conversion operator

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: llvm/lib/Demangle/MicrosoftDemangle.cpp:248-249
 StringView Demangler::copyString(StringView Borrowed) {
-  char *Stable = Arena.allocUnalignedBuffer(Borrowed.size() + 1);
-  std::strcpy(Stable, Borrowed.begin());
+  char *Stable = Arena.allocUnalignedBuffer(Borrowed.size());
+  std::memcpy(Stable, Borrowed.begin(), Borrowed.size());
 
----------------
can this change be made independent of the rest of the patch? If so, it'd be good to separate it out.


================
Comment at: llvm/lib/Demangle/MicrosoftDemangle.cpp:973
   Identifier->output(OB, OF_Default);
-  OB << '\0';
-  char *Name = OB.getBuffer();
-
-  StringView Owned = copyString(Name);
+  StringView Owned = copyString(StringView(OB));
   memorizeString(Owned);
----------------
Since the conversion operator is implicit, could the `StringView(``)` be omitted here?


================
Comment at: llvm/lib/Demangle/MicrosoftDemangle.cpp:2318
 
-    std::printf("  [%d] - %.*s\n", (int)I, (int)OB.getCurrentPosition(),
-                OB.getBuffer());
+    StringView B(OB);
+    std::printf("  [%d] - %.*s\n", (int)I, (int)B.size(), B.begin());
----------------
I'd tend to recommend writing this as `StringView B = OB` if that's valid (since that syntax only allows implicit conversions - so it simplifies things for readers (they have to consider fewer possible conversion sequences))


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

https://reviews.llvm.org/D120990