This is an archive of the discontinued LLVM Phabricator instance.

llvm/utils/benchmark: add missing <limits> inclusion
AbandonedPublic

Authored by trofi on Oct 15 2020, 12:09 AM.

Details

Summary

Noticed missing header when was building llvm with gcc-11:

llvm-project/llvm/utils/benchmark/src/benchmark_register.h:17:30:
  error: 'numeric_limits' is not a member of 'std'
   17 |   static const T kmax = std::numeric_limits<T>::max();
      |                              ^~~~~~~~~~~~~~

Signed-off-by: Sergei Trofimovich <slyfox@inbox.ru>

Diff Detail

Event Timeline

trofi created this revision.Oct 15 2020, 12:09 AM
Herald added a project: Restricted Project. · View Herald Transcript
trofi requested review of this revision.Oct 15 2020, 12:09 AM

Right, but can you please submit this upstream first, so it doesn't get lost?

kbobyrev requested changes to this revision.Oct 15 2020, 1:07 AM

Thanks! After it is accepted in upstream benchmark, please add an entry to llvm/utils/benchmark/README.llvm which contains information about cherry-picked commits that are applied on top of the local copy.

This revision now requires changes to proceed.Oct 15 2020, 1:07 AM

Thanks! After it is accepted in upstream benchmark, please add an entry to llvm/utils/benchmark/README.llvm which contains information about cherry-picked commits that are applied on top of the local copy.

To add to that, there are three copies of benchmark - one here, one in libcxx, and one in separate test-suite repo.
It might be good to patch all three.

That's true and I was thinking about doing that at some point since it especially makes sense after the monorepo emerging on GitHub but I can't find enough time yet :( If anyone is willing to contribute or lead the way I'd be happy to review and help but I might not be able to do that myself right ntow.

nvieen added a subscriber: nvieen.Mar 9 2023, 2:47 PM

That’s this: ⚙ D89450 llvm/utils/benchmark: add missing <limits> inclusion You can just add #include <limits> for now; the next llvm upgrade should fix this.
for that specific version of llvm at 100%

Herald added a project: Restricted Project. · View Herald TranscriptMar 9 2023, 2:47 PM
Herald added a subscriber: StephenFan. · View Herald Transcript