[Lldb-commits] [PATCH] D100962: [lldb/Core] Add SourceLocationSpec class (NFC)

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

A few more comments about things I missed in the previous review.



================
Comment at: lldb/source/Core/SourceLocationSpec.cpp:57
+
+void SourceLocationSpec::Dump(llvm::raw_ostream &s) const {
+  StreamString ss;
----------------
Should this take a `StreamString` so we can write to the stream directory? Or alternatively change the dump method in declaration to also take a `raw_ostream`? Both would also improve the `operator<<`.


================
Comment at: lldb/source/Core/SourceLocationSpec.cpp:65-71
+const char *SourceLocationSpec::GetCString() const {
+  std::string data;
+  llvm::raw_string_ostream ss(data);
+  Dump(ss);
+
+  return ConstString{ss.str()}.AsCString();
+}
----------------
This should probably return a `std::string` instead. Now we're going to keep the string around forever for every invocation of this method. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100962