[libcxx-commits] [PATCH] D108321: [SystemZ][z/OS] Avoid assumption for character value in futures tests

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone accepted this revision as: Quuxplusone.
Quuxplusone added inline comments.


================
Comment at: libcxx/test/std/thread/futures/futures.task/futures.task.members/operator.pass.cpp:90
         support::make_test_thread(func0, std::move(p)).detach();
-        assert(f.get() == 105.0);
+        assert(f.get() == 8.0 + 'a');
     }
----------------
muiez wrote:
> Quuxplusone wrote:
> > I suggest that a much //nicer// way to fix all these tests would be to eliminate the `(int, char)` signature and the cast-from-char-to-double entirely. Just have it take `double(int, float)` or something, and pass `4.0` instead of `'a'`.
> > 
> > Slightly less invasive: Leave the `(int, char)` signature, but pass `97` instead of `'a'`.
> > 
> > More invasive, and "clever," but clean-looking: Leave the signature, pass `'0'` instead of `'a'`, and check that the result is `(double)'8'`... no, wait, that would be really dumb. Don't do that. :)
> > 
> > Either way, please upload with full context: use `git show -U999` or `git diff -U999` or whatever the `arc` incantation is.
> Thanks for the suggestion. Please see updated diff :) 
LGTM now. Of course it's now utterly bizarre that we're choosing magic numbers like `97`, `99`, `122` instead of, say, `1`, `2`, `3`... but that's a pre-existing weirdness of these tests. I think this is a good solution at this point.


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

https://reviews.llvm.org/D108321