[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 11:43:08 PDT 2017


MatzeB added a comment.

- Adding a mode to ignore whitespace seems reasonable.
- I remember seeing fpcmp loop endlessly and never got around figuring out why, good to see this fixed.



================
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;
----------------
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.


================
Comment at: tools/fpcmp.c:297-298
   if (A_size == 0 || B_size == 0) {
     fprintf(stderr, "%s File sizes differ\n", g_program);
     return 1;
   }
----------------
Wouldn't we abort here when comparing an empty file with a file with just spaces even in `-i` mode?


https://reviews.llvm.org/D36768





More information about the llvm-commits mailing list