[PATCH] D85131: [NFC][APInt][DenseMapInfo] Move DenseMapAPIntKeyInfo into DenseMap.h as DenseMapInfo<APInt>

[Kuter Dinel via Phabricator via llvm-commits]

kuter added inline comments.


================
Comment at: llvm/include/llvm/ADT/DenseMapInfo.h:352
+/// Provide DenseMapInfo for APInt.
+template <> struct DenseMapInfo<APInt> {
+  static inline APInt getEmptyKey() {
----------------
okura wrote:
> kuter wrote:
> > It seems that,  it is possible to specify arbitrary key info traits for `DenseSet`. 
> > This was what `DenseMapAPIntKeyInfo` in `LLVMContextImpl.h` was dong.
> > It would declare a `DenseSet` like this:
> > `DenseSet<APInt, DenseMapAPIntKeyInfo>`
> > 
> > This specialization   might not be desirable for every use case since `static_cast<unsigned>` discards information. 
> > 
> > Maybe we want to do this in the Attributor like the way `LLVMContextImpl.h` was doing it ?
> Sorry, I didn't understand what you mean. 
> 
> > This specialization might not be desirable for every use case since static_cast<unsigned> discards information.
> This specialization has already been in `LLVMContextImpl.h` and I didn't change anything.
> 
> I want to use it in Attributor, but it exists under `/lib` directory and we were not able to include it directly.
> So I moved it into here.
What I was saying is that you made this the default for `APInt`.
I wasn't exactly sure that this should be the default.

I was talking about keeping it as `DenseMapAPIntKeyInfo`.
I wrote this before this revision got accepted.



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85131/new/

https://reviews.llvm.org/D85131