[PATCH] D148348: [StringView] remove dropFront

[Erich Keane via Phabricator via llvm-commits]

erichkeane added a comment.

So I think most of your asserts are actually unnecessary.  I only made it about 1/2 way down, but dont see any that are obviously necessary.



================
Comment at: llvm/include/llvm/Demangle/ItaniumDemangle.h:1841
       OB += "-";
-      OB += Offset.dropFront();
+      assert(1 <= Offset.size());
+      OB += Offset.substr(1);
----------------
I think this assert is actually ALSO unnecessary, we already know that we're not in `Offset.empty`, line 1837 (and even if we weren't, line 1839 would probably be UB?).


================
Comment at: llvm/include/llvm/Demangle/ItaniumDemangle.h:2635
                "operator name does not start with 'operator'");
-        Res = Res.dropFront(sizeof("operator") - 1);
+        assert(sizeof("operator") - 1 <= Res.size());
+        Res = Res.substr(sizeof("operator") - 1);
----------------
See above assert that checks startsWith, so also probably unnecessary.


================
Comment at: llvm/include/llvm/Demangle/ItaniumDemangle.h:3710
     if (Qual.startsWith("objcproto")) {
-      StringView ProtoSourceName = Qual.dropFront(std::strlen("objcproto"));
+      assert(std::strlen("objcproto") <= Qual.size());
+      StringView ProtoSourceName = Qual.substr(std::strlen("objcproto"));
----------------
This assert is also redundant.


================
Comment at: llvm/lib/Demangle/MicrosoftDemangle.cpp:172
+  assert(1 <= Candidate.size());
+  Candidate = Candidate.substr(1);
   while (!Candidate.empty()) {
----------------
index of zero above these makes this assert 'too late', since it would already be UB.


================
Comment at: llvm/lib/Demangle/MicrosoftDemangle.cpp:491
   assert(MangledName.startsWith('?'));
-  MangledName.remove_prefix(1);
+  assert(1 <= MangledName.size());
+  MangledName = MangledName.substr(1);
----------------
startswith above this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148348