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

[Dávid Bolvanský via Phabricator via llvm-commits]

xbolva00 added inline comments.


================
Comment at: llvm/trunk/include/llvm/Demangle/ItaniumDemangle.h:609
+      assert(FirstChar == SecondChar);
+      ++FirstChar, ++SecondChar;
+    }
----------------
erik.pilkington wrote:
> 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. 
Thanks for info. Can you add code comment here for now?

 


Repository:
  rL LLVM

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

https://reviews.llvm.org/D50930