[flang] Fix compile error : on MacOS 11.5.1 (20G80), cmake version 3.21.1 #333
Conversation
…21.1. error: constant expression evaluates to -1 which cannot be narrowed to type 'std::clock_t' (aka 'unsigned long') [-Wc++11-narrowing]
|
This repository does not accept pull requests. Please follow http://llvm.org/docs/Contributing.html#how-to-submit-a-patch for contribution to LLVM. |
There was a problem hiding this comment.
Thanks for this patch. Since reviews happen in phabricator (http://llvm.org/docs/Contributing.html#how-to-submit-a-patch), will you be making a patch there?
| @@ -26,6 +26,10 @@ | |||
| // overload will have a dummy parameter whose type indicates whether or not it | |||
| // should be preferred. Any other parameters required for SFINAE should have | |||
| // default values provided. | |||
|
|
|||
| // #define invalid_clock_t (-1) | |||
There was a problem hiding this comment.
Nit: Remove commented code.
| @@ -26,6 +26,10 @@ | |||
| // overload will have a dummy parameter whose type indicates whether or not it | |||
| // should be preferred. Any other parameters required for SFINAE should have | |||
| // default values provided. | |||
|
|
|||
| // #define invalid_clock_t (-1) | |||
| #define INVALID_CLOCK_T (0xFF) | |||
There was a problem hiding this comment.
Will 0xFF work on machines that have std::clock_t as a signed integer? Will a static cast of -1 to std::clock_t work better?
There was a problem hiding this comment.
You definitely don't want 0xFF, as that is a "valid" clock_t value (although it would be very close to January 1st, 1970 :) ). My local patch was to use static_cast<clock_t>(-1) instead.
There was a problem hiding this comment.
Btw, on FreeBSD the situation is 'interesting' in the sense that clock_t is unsigned only on 32-bit architectures (probably for historical reasons), and signed on everything else. On macOS it seems that __darwin_clock_t is always just unsigned long.
There was a problem hiding this comment.
you might consider std::numeric_limits<std::clock_t>::max(), which will be platform independent, and pretty clear what is happening.
There was a problem hiding this comment.
you might consider
std::numeric_limits<std::clock_t>::max(), which will be platform independent, and pretty clear what is happening.
No, you should really use -1. The clock(3) man pages on both macOS and FreeBSD say:
RETURN VALUES
The clock() function returns the amount of time used unless an error occurs, in which case the return value is -1.
On Linux:
RETURN VALUE
The value returned is the CPU time used so far as a clock_t; to get the number of seconds used, divide by CLOCKS_PER_SEC. If the processor time used is not available or its value cannot be represented, the func‐
tion returns the value (clock_t) -1.
I think the Linux man page makes it a bit clearer that you have to cast -1 to clock_t.
There was a problem hiding this comment.
(Having said all that, it's very unlikely that you cannot retrieve the current time.... but still, better safe than sorry :) )
|
I created a patch (https://reviews.llvm.org/D107972) in phabricator. |
[flang] Fix compile error : on MacOS 11.5.1 (20G80), cmake version 3.21.1
error: constant expression evaluates to -1 which cannot be narrowed to type 'std::clock_t' (aka 'unsigned long') [-Wc++11-narrowing]