[PATCH] D50930: Move Itanium demangler implementation into a header file and add visitation support.

[Erik Pilkington via Phabricator via llvm-commits]

erik.pilkington added inline comments.


================
Comment at: llvm/trunk/include/llvm/Demangle/ItaniumDemangle.h:609
+      assert(FirstChar == SecondChar);
+      ++FirstChar, ++SecondChar;
+    }
----------------
xbolva00 wrote:
> Hi Richard, this code looks suspicious. Is this correct?
> 
> PVS warns:
> /home/xbolva00/LLVM/llvm-project/llvm/include/llvm/Demangle/ItaniumDemangle.h	620	err	V769 The 'SecondChar' pointer in the '++ SecondChar' expression equals nullptr. The resulting value is senseless and it should not be used.
> 
> 
(This is my fault, Richard just copied this)

This is sorta fine, this done to distinguish between an empty StringView with nullptr first and last and an empty NodeOrString. The pointers are never loaded from, but I believe this is still UB. That being said, I think this class is over engineered and we'd probably be better off putting the StringView in a `NameType` in cases where we need to have either a Node or a String. 


Repository:
  rL LLVM

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

https://reviews.llvm.org/D50930