RESOLVED FIXED 164991
std::isnan/isinf/isfinite should work with WTF time classes
https://bugs.webkit.org/show_bug.cgi?id=164991
Summary std::isnan/isinf/isfinite should work with WTF time classes
Filip Pizlo
Reported 2016-11-19 11:41:07 PST
Patch forthcoming.
Attachments
the patch (4.79 KB, patch)
2016-11-19 11:44 PST, Filip Pizlo
darin: review+
possible alternate approach (17.19 KB, patch)
2017-01-10 16:12 PST, Filip Pizlo
no flags
Filip Pizlo
Comment 1 2016-11-19 11:44:12 PST
Created attachment 295266 [details] the patch
Darin Adler
Comment 2 2016-11-19 12:40:32 PST
Comment on attachment 295266 [details] the patch Maybe isfinite too?
Filip Pizlo
Comment 3 2017-01-10 15:48:54 PST
Dropped this one on the floor. Coming back to it now.
Filip Pizlo
Comment 4 2017-01-10 16:12:13 PST
Created attachment 298520 [details] possible alternate approach
Filip Pizlo
Comment 5 2017-01-10 16:17:34 PST
Comment on attachment 298520 [details] possible alternate approach Clear r? to reflect the fact that I still need to add unit tests, and doing so will probably unearth bugs in how I wrote my enable_if constraints.
Darin Adler
Comment 6 2017-01-10 21:07:30 PST
I like both of these. I normally judge such things by how good they look at call sites, how good the error messages are when you use them wrong, whether there are any cases where they do something surprising when you use them wrong, and how portable the technique is. I don’t have a preference between the two approaches in this bug given these criteria, so I would be tempted to use the more straightforward approach until things got complex enough that the traits solution really started to pay off.
Filip Pizlo
Comment 7 2017-03-09 12:46:00 PST
Comment on attachment 298520 [details] possible alternate approach I'll go with my original approach.
Filip Pizlo
Comment 8 2017-03-09 12:53:17 PST
I'm going to land this with isfinite.
Filip Pizlo
Comment 9 2017-03-09 13:19:02 PST
Note You need to log in before you can comment on or make changes to this bug.