[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