[PATCH] D36768: [test-suite] Add -i option to fpcmp to ignore whitespace changes.

Matthias Braun via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Aug 17 16:33:24 PDT 2017


MatzeB accepted this revision.
MatzeB added a comment.
This revision is now accepted and ready to land.

This is looking good! I am wondering if one of the if blocks is still necessary (see below).



================
Comment at: tools/fpcmp.c:260
+      // Without this check, we would be in an infinite loop.
+      if (!ignore_whitespace && isspace(*F1P) && isspace(*F2P))
+        return 1;
----------------
MatzeB wrote:
> You mustn't pass `char` to libc character test functions but must cast characters to `unsigned char` (this is so intuitive, isn't it?). I guess you could also fix the existing isspace uses in the file while on it...
> 
> I wonder if this isn't a case that should rather be handled in CompareNumber.
Is the `if (!ignore_whitespace) { ... }` still necessary after your changes to `CompareNumbers`?


https://reviews.llvm.org/D36768





More information about the llvm-commits mailing list