This is an archive of the discontinued LLVM Phabricator instance.

[llvm-readobj][NFC] Replace size_t in HashHistogram with uint32_t
AbandonedPublic

Authored by paulkirth on Mar 21 2023, 10:06 AM.

Details

Summary

Someone ran into an issue with a size_t interface on Darwin.
see (https://reviews.llvm.org/D146492).

Given the use case for HashHistograms, a 32bit unsigned integer should
be sufficient to represent all computed values without overflow.

Diff Detail

Event Timeline

paulkirth created this revision.Mar 21 2023, 10:06 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 21 2023, 10:06 AM
paulkirth requested review of this revision.Mar 21 2023, 10:06 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 21 2023, 10:06 AM
junhee-yoo added inline comments.Mar 21 2023, 5:37 PM
llvm/tools/llvm-readobj/ELFDumper.cpp
7181

I guess if the type of I is changed from size_t to uint32_t, static_cast<uint64_t> is no longer needed here and on Count[I] because virtual void printNumber(StringRef Label, uint32_t Value) is exist. How do you think?

I've got nothing against this change, but see my comment in D146492.

paulkirth abandoned this revision.Apr 21 2023, 9:43 AM

I think this has become obsolete now that we handle fundamental C++ types.