teemperor accepted this revision.
teemperor added a comment.

Thanks for updating and thanks for the patch! I can land it right now.



================
Comment at: lldb/unittests/Core/FormatEntityTest.cpp:154
+TEST(FormatEntity, LookupAllEntriesInTree) {
+  for (const auto &testString : lookupStrings) {
+    Entry e;
----------------
nealsid wrote:
> teemperor wrote:
> > teemperor wrote:
> > > You could just use `llvm::StringRef` here (it's shorter and more 
> > > descriptive than the `auto`).
> > I actually meant `llvm::StringRef` as the whole type (StringRef references 
> > are not really useful as StringRef is already a lightweight String 
> > reference)
> Done - kept it const since StringRef appears to depend on that for some of 
> its methods.  I was looking through StringRef (and the use of optionals for 
> something else) and as far as I understand, which could be entirely wrong, 
> it's meant to provide a single interface over unowned character data whether 
> it comes from a std::string or char*, and thought using string_view would be 
> good, as well - is moving to C++17 on the roadmap? I know it's one of those 
> things that everyone is in favor of and wants to do but nobody has time for :)
> 
It's on the roadmap but not sure when that will happen. I think that depends on 
when the interest in having C++17 features is bigger than the pain it will 
inflict on people stuck on outdated runtime environments that need 
bootstrapping for C++17.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98153

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to