This is an archive of the discontinued LLVM Phabricator instance.

[ADT] Move DenseMapInfo for APInt into APInt.h (PR50527)
ClosedPublic

Authored by nikic on May 31 2021, 11:31 AM.

Details

Summary

As suggested in https://bugs.llvm.org/show_bug.cgi?id=50527, this moves the DenseMapInfo for APInt and APSInt into the respective headers, removing the need to include APInt.h and APSInt.h from DenseMapInfo.h.

We could probably do the same from StringRef and ArrayRef as well.

Diff Detail

Event Timeline

nikic created this revision.May 31 2021, 11:31 AM
nikic requested review of this revision.May 31 2021, 11:31 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 31 2021, 11:31 AM
nikic edited the summary of this revision. (Show Details)May 31 2021, 11:32 AM
craig.topper added inline comments.
llvm/lib/Support/APInt.cpp
553

Why is this out of line now?

nikic added inline comments.Jun 1 2021, 12:16 AM
llvm/lib/Support/APInt.cpp
553

It avoids the need to include Hashing.h in APInt.h for the hash_code type.

lattner accepted this revision.Jun 1 2021, 8:46 AM

Thank you so much for tackling this!

llvm/lib/Support/APInt.cpp
553

I think this is fine - the implementation of hash_value is out of line just a few lines above anyway, so the whole function isn't going to be inlined in either case.

This revision is now accepted and ready to land.Jun 1 2021, 8:46 AM
This revision was landed with ongoing or failed builds.Jun 1 2021, 9:32 AM
This revision was automatically updated to reflect the committed changes.

We could probably do the same from StringRef and ArrayRef as well.

@nikic Are you intending to address these as well?

nikic added a comment.Jun 1 2021, 1:50 PM

We could probably do the same from StringRef and ArrayRef as well.

@nikic Are you intending to address these as well?

Done in D103491.