[Lldb-commits] [PATCH] D148546: Reland: [Demangle] replace use of llvm::StringView w/ std::string_view

[Nick Desaulniers via Phabricator via lldb-commits]

nickdesaulniers marked 2 inline comments as done.
nickdesaulniers added a comment.

In D148546#4276926 <https://reviews.llvm.org/D148546#4276926>, @DavidSpickett wrote:

> I am confused why startswith is in the itanium demangle namespace but I could be confusing a specialised function with the generic one that works for any string view. Otherwise looks fine at a glance.

Hey David! Thanks for the review!  See the two parent commits for more context:

1. https://reviews.llvm.org/D148547
2. https://reviews.llvm.org/D148556

The second in particular could use review still ;) The first has landed.  This patch is part of a stack that could use review if you have the cycles for it.



================
Comment at: llvm/include/llvm/Demangle/MicrosoftDemangle.h:14
 
+#include <cassert>
+#include <string_view>
----------------
DavidSpickett wrote:
> Needed because llvm's stringview included assert?
yep, used below on L49.


================
Comment at: llvm/include/llvm/Demangle/Utility.h:19
 
-#include "StringView.h"
+#include "DemangleConfig.h"
+
----------------
DavidSpickett wrote:
> Why does this change?
The definition of DEMANGLE_NAMESPACE_BEGIN used below on L30 comes from this header, which was being included indirectly by StringView.h before this change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148546