[PATCH] D112105: [ADT] Simplifying hex string parsing so it runs faster in debug modes.

Ben Vanik via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 21 09:46:32 PDT 2021


benvanik added inline comments.


================
Comment at: llvm/unittests/ADT/StringExtrasTest.cpp:93
 
-  std::string InvalidStr = "A5ZX";
+  std::string InvalidStr = "A50\xFF";
   std::string IgnoredOutput;
----------------
benvanik wrote:
> dblaikie wrote:
> > If this is purely a performance improvement - what's motivating this test change?
> I don't like undefined behavior and wanted to verify my new code handled this case correctly. Prior to this the behavior was undefined if you had any ascii character with value 255 in the input file, which I think can be agreed is not good regardless of whether most normal inputs include it or not.
> 
> (I'm curious why the question: is updating tests to improve coverage for fixed bugs something that needs extrinsic motivation? I did call out in the description that I fixed this bug as part of this refactoring)
(to clarify: this does not reduce test coverage as this was just testing that invalid characters were correctly identified as invalid - I just changed it so the invalid character it was testing was 0xFF)


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

https://reviews.llvm.org/D112105



More information about the llvm-commits mailing list