[PATCH] D83283: [Attributor] AAPotentialValues Interface

[Shinji Okumura via Phabricator via llvm-commits]

okura added inline comments.


================
Comment at: llvm/include/llvm/ADT/APInt.h:2293
+/// Provide DenseMapInfo for APInt.
+template <> struct DenseMapInfo<APInt> {
+  static inline APInt getEmptyKey() {
----------------
fhahn wrote:
> I am not sure this is the right place. Shouldn't that go into DenseMapInfo.h? Also, if this gets added there, it should probably be split off from this patch.
I'm sorry. I will move this into DenseMapInfo.h in a new patch.


================
Comment at: llvm/include/llvm/Transforms/IPO/Attributor.h:3455
+  static bool classof(const AbstractAttribute *AA) {
+    return (AA->getIdAddr() == &ID);
+  }
----------------
fhahn wrote:
> okura wrote:
> > fhahn wrote:
> > > nit: unnecessary ()
> > These brackets are unnecessary indeed, but this function is one of the boilerplate functions of AbstactAttributes and the other AAs have the same implementation. So I think we don't need to remove them.
> FWIW, I think this is very uncommon in most parts of the llvm codebase.
I think so too. I will make a new patch to remove these brackets in all AbstractAttriutes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83283