nickdesaulniers added inline comments.
================ Comment at: llvm/lib/Demangle/Demangle.cpp:31 std::string Result; - const char *S = MangledName.c_str(); + std::string Copy = MangledName.str(); + const char *S = Copy.data(); ---------------- erichkeane wrote: > nickdesaulniers wrote: > > nickdesaulniers wrote: > > > erichkeane wrote: > > > > `std::string` is implicitly constructible from a `std::string_view`, so > > > > I think on line 37 we can just do: `MangledName[0]` instead of `S[0]`, > > > > and just do `return MangledName` below, rather than constructing it > > > > every time, despite it being just destructed in every other branch. > > > > > > > > Though, I guess the 'gotcha' here is that despite everything else not > > > > modifying the `S` here, that we're no longer able to use strlen for the > > > > length. Perhaps this change should be more viral, in that the other > > > > demangle functions should take a `string_view` instead of a `const char > > > > *` as well. > > > > Perhaps this change should be more viral, in that the other demangle > > > > functions should take a string_view instead of a const char * as well. > > > > > > Sigh, include/llvm/Demangle/StringView.h has a comment > > > > > > 9 // FIXME: Use std::string_view instead when we support C++17. > > > > > > > > > and is used throughout llvm/lib/Demangle/. This is potentially a large > > > cleanup. Let me start on that... > > Not having starts_with and ends_with until C++20 is going to be a PITA. > Ouch, yeah, i could see that being a problem. Perhaps we need an > `llvm::starts_with`/`llvm::ends_with` that implements in terms of substr? Yeah, I guess llvm/ADT/STLExtras.h might be an appropriate place to add them. So far in the conversion, another pain point has been llvm::StringView's constructor that takes two char*'s. I think to break up this conversion, I might remove that constructor and modify the callsites. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148181/new/ https://reviews.llvm.org/D148181 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
