This is an archive of the discontinued LLVM Phabricator instance.

[flang] Compile error instantiating std::clock_t on Apple clang version 11.0.0
ClosedPublic

Authored by craig.rasmussen on Jul 7 2021, 2:51 PM.

Details

Summary

Apple clang gives error with initialization std::clock_t{-1}.

Error message is

/Users/rasmussen17/Compilers/llvm-project/flang/runtime/time-intrinsic.cpp:39:33: error: constant expression

    evaluates to -1 which cannot be narrowed to type 'std::clock_t' (aka 'unsigned long') [-Wc++11-narrowing]
if (timestamp != std::clock_t{-1}) {
                              ^~

Simple fix is to replace with a static cast: std::clock_t{static_cast<clock_t>(-1)}

Diff Detail

Event Timeline

craig.rasmussen created this revision.Jul 7 2021, 2:51 PM
craig.rasmussen requested review of this revision.Jul 7 2021, 2:51 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 7 2021, 2:51 PM
craig.rasmussen edited the summary of this revision. (Show Details)Jul 7 2021, 2:57 PM
jhenderson added a comment.EditedJul 8 2021, 12:38 AM

It looks like you've got many changes in this diff that are unrelated? Have you rebased properly and generated your diff accordingly?

craig.rasmussen removed a reviewer: jhenderson.

Forgot to commit changes so patch/diff was not correct.

Fixed patch and added reviewer.

rovka added a subscriber: rovka.Jul 12 2021, 11:07 PM
rovka added inline comments.
flang/runtime/time-intrinsic.cpp
39

Can we simplify to this instead?

timestamp != static_cast<std::clock_t>(-1)

Made simplification suggested by reviewer:

std::clock_t{static_cast<clock_t>(-1)} --> static_cast<clock_t>(-1)

craig.rasmussen marked an inline comment as done.Jul 21 2021, 4:06 PM

Requesting review (is there a better way to request a review)

rovka accepted this revision.Jul 26 2021, 8:42 AM

LGTM, thanks!

This revision is now accepted and ready to land.Jul 26 2021, 8:42 AM
rovka added a comment.Jul 26 2021, 8:45 AM

Requesting review (is there a better way to request a review)

I think people usually notice and have another look when you upload a new version of the patch. If there's no reaction, common practice is to ping about once a week.

How do I ping? I have another pull request that is also in limbo.

Thanks,
Craig

From: Diana Picus via Phabricator <reviews@reviews.llvm.org>
Date: Monday, July 26, 2021 at 8:45 AM
To: Rasmussen, Craig E. <rasmussen17@llnl.gov>, jh7370@my.bristol.ac.uk <jh7370@my.bristol.ac.uk>, sscalpone@nvidia.com <sscalpone@nvidia.com>, diana.picus@linaro.org <diana.picus@linaro.org>
Cc: maskray@google.com <maskray@google.com>, llvm-commits@lists.llvm.org <llvm-commits@lists.llvm.org>, mgorny@gentoo.org <mgorny@gentoo.org>, rupprecht@google.com <rupprecht@google.com>, bhuvanendra.kumarn@amd.com <bhuvanendra.kumarn@amd.com>, yanliang.mu@intel.com <yanliang.mu@intel.com>, dougpuob@gmail.com <dougpuob@gmail.com>, tim@tkeith.com <tim@tkeith.com>, kuhnel@google.com <kuhnel@google.com>
Subject: [PATCH] D105595: [flang] Compile error instantiating std::clock_t on Apple clang version 11.0.0
rovka added a comment.

In D105595#2894946 <https://urldefense.us/v3/__https://reviews.llvm.org/D105595*2894946__;Iw!!G2kpM7uM-TzIFchu!j8TcPSbVUpRWmsqLEU_2NiAJlQkuOPOamifPnxDhHpBXqKbAyypxYlbd08Z4MU1Ca6WH$ >, @craig.rasmussen wrote:

Requesting review (is there a better way to request a review)

I think people usually notice and have another look when you upload a new version of the patch. If there's no reaction, common practice is to ping about once a week.

CHANGES SINCE LAST ACTION

https://urldefense.us/v3/__https://reviews.llvm.org/D105595/new/__;!!G2kpM7uM-TzIFchu!j8TcPSbVUpRWmsqLEU_2NiAJlQkuOPOamifPnxDhHpBXqKbAyypxYlbd08Z4MWbf3AVF$<https://urldefense.us/v3/__https:/reviews.llvm.org/D105595/new/__;!!G2kpM7uM-TzIFchu!j8TcPSbVUpRWmsqLEU_2NiAJlQkuOPOamifPnxDhHpBXqKbAyypxYlbd08Z4MWbf3AVF$>

https://urldefense.us/v3/__https://reviews.llvm.org/D105595__;!!G2kpM7uM-TzIFchu!j8TcPSbVUpRWmsqLEU_2NiAJlQkuOPOamifPnxDhHpBXqKbAyypxYlbd08Z4MfUDIKSg$https://urldefense.us/v3/__https:/reviews.llvm.org/D105595__;!!G2kpM7uM-TzIFchu!j8TcPSbVUpRWmsqLEU_2NiAJlQkuOPOamifPnxDhHpBXqKbAyypxYlbd08Z4MfUDIKSg$

How do I ping? I have another pull request that is also in limbo.

Just literally post a comment saying "ping" or similar to grab people's attention.

rovka closed this revision.Aug 19 2021, 12:07 AM

Hi Craig,

I think there was some confusion related to this patch. I approved it long ago, which means that you could commit it upstream. See the docs explaining the review process. Since you didn't commit it or ask someone else to do it, it was fixed independently in the meantime. Unfortunately I was out of office during the time so I didn't catch it. In any case, thanks for the patch and next time please let people know during the review if you don't have commit rights.