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 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
