[PATCH] D150997: [llvm] Split out DenseMapInfo specialization

This revision was automatically updated to reflect the committed changes. Closed by commit rGcea0eea28e71: [llvm] Split out DenseMapInfo specialization (authored by IncludeGuardian). Herald added a project: Flang. Changed prior to commit: https://reviews.llvm.org/D150997?vs=532038&id=5

[PATCH] D150997: [llvm] Split out DenseMapInfo specialization

nikic accepted this revision. nikic added a comment. This revision is now accepted and ready to land. LGTM. The diff looks weird for some reason, but the downloadable `.diff` file looks fine. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150997/new/ https://reviews.llvm.org/D150997 __

[PATCH] D150997: [llvm] Split out DenseMapInfo specialization

IncludeGuardian added a comment. This is good to review now. Phabricator has become a bit confused after rebasing on top of https://reviews.llvm.org/D151557, I don't know whether it's worth creating a new, cleaner review and close this on? CHANGES SINCE LAST ACTION https://reviews.llvm.org/

[PATCH] D150997: [llvm] Split out DenseMapInfo specialization

IncludeGuardian updated this revision to Diff 532038. IncludeGuardian added a comment. Run clang-format over patch CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150997/new/ https://reviews.llvm.org/D150997 Files: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h

[PATCH] D150997: [llvm] Split out DenseMapInfo specialization

IncludeGuardian updated this revision to Diff 531895. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150997/new/ https://reviews.llvm.org/D150997 Files: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h flang/include/flang/Optimizer/HLFIR/HLFIROps.h llvm/includ

[PATCH] D150997: [llvm] Split out DenseMapInfo specialization

IncludeGuardian updated this revision to Diff 531060. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150997/new/ https://reviews.llvm.org/D150997 Files: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h flang/include/flang/Optimizer/HLFIR/HLFIROps.h llvm/includ

[PATCH] D150997: [llvm] Split out DenseMapInfo specialization

IncludeGuardian updated this revision to Diff 530855. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150997/new/ https://reviews.llvm.org/D150997 Files: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h flang/include/flang/Optimizer/HLFIR/HLFIROps.h llvm/includ

[PATCH] D150997: [llvm] Split out DenseMapInfo specialization

IncludeGuardian updated this revision to Diff 530819. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150997/new/ https://reviews.llvm.org/D150997 Files: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h flang/include/flang/Optimizer/HLFIR/HLFIROps.h llvm/includ

[PATCH] D150997: [llvm] Split out DenseMapInfo specialization

IncludeGuardian updated this revision to Diff 530787. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150997/new/ https://reviews.llvm.org/D150997 Files: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h flang/include/flang/Optimizer/HLFIR/HLFIROps.h llvm/includ

[PATCH] D150997: [llvm] Split out DenseMapInfo specialization

IncludeGuardian updated this revision to Diff 530667. IncludeGuardian added a comment. Fix missing includes in test, fix formatting, and rebase on top of https://reviews.llvm.org/D151557 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150997/new/ https://reviews.llvm.org/D150997 Files:

[PATCH] D150997: [llvm] Split out DenseMapInfo specialization

nikic requested changes to this revision. nikic added a comment. This revision now requires changes to proceed. (Moving this off the review queue per above comments. There is a missing include in a unit test.) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150997/new/ https://reviews.ll

[PATCH] D150997: [llvm] Split out DenseMapInfo specialization

IncludeGuardian added a comment. @kadircet Thanks I will run this through the formatter. Thanks for fixing `isEqual`. I am currently on holiday for a few weeks so cannot guarantee when I'll be able to update this PR. I say go ahead with your fix and I'll incorporate those changes into this revi

[PATCH] D150997: [llvm] Split out DenseMapInfo specialization

kadircet added a comment. In D150997#4357589 , @nikic wrote: > It looks like you forgot to `git add` the new file maybe? > > This was added in https://reviews.llvm.org/D133200, maybe @kadircet can > comment on what this was intended for. It was already

[PATCH] D150997: [llvm] Split out DenseMapInfo specialization

kadircet added a comment. In D150997#4357589 , @nikic wrote: > It looks like you forgot to `git add` the new file maybe? > > This was added in https://reviews.llvm.org/D133200, maybe @kadircet can > comment on what this was intended for. It was already

[PATCH] D150997: [llvm] Split out DenseMapInfo specialization

barannikov88 added inline comments. Comment at: flang/include/flang/Optimizer/HLFIR/HLFIROps.h:22 #include "mlir/Interfaces/SideEffectInterfaces.h" +#include IncludeGuardian wrote: > I wasn't sure if it is possible to include `` within `HLFIROps.td` so > it

[PATCH] D150997: [llvm] Split out DenseMapInfo specialization

IncludeGuardian requested review of this revision. IncludeGuardian added inline comments. Comment at: flang/include/flang/Optimizer/HLFIR/HLFIROps.h:22 #include "mlir/Interfaces/SideEffectInterfaces.h" +#include I wasn't sure if it is possible to include `` w

[PATCH] D150997: [llvm] Split out DenseMapInfo specialization

IncludeGuardian updated this revision to Diff 524125. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150997/new/ https://reviews.llvm.org/D150997 Files: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h flang/include/flang/Optimizer/HLFIR/HLFIROps.h llvm/includ

[PATCH] D150997: [llvm] Split out DenseMapInfo specialization

IncludeGuardian updated this revision to Diff 524109. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150997/new/ https://reviews.llvm.org/D150997 Files: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h llvm/include/llvm/ADT/DenseMapInfo.h llvm/include/llvm/ADT

[PATCH] D150997: [llvm] Split out DenseMapInfo specialization

IncludeGuardian updated this revision to Diff 524094. Herald added subscribers: bviyer, Moerafaat, zero9178, bzcheeseman, sdasgup3, wenzhicui, wrengr, cota, teijeong, rdzhabarov, tatianashp, msifontes, jurahul, Kayjukh, grosul1, Joonsoo, stephenneuendorffer, liufengdb, aartbik, mgester, arpith-j

[PATCH] D150997: [llvm] Split out DenseMapInfo specialization

lattner accepted this revision. lattner added a comment. This revision is now accepted and ready to land. Awesome, this makes a lot of sense and is a great pattern to make these more modular. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150997/new/ https://reviews.llvm.org/D150997 __

[PATCH] D150997: [llvm] Split out DenseMapInfo specialization

IncludeGuardian updated this revision to Diff 524041. IncludeGuardian added a comment. Add missing specialization for clang-include-cleaner/Types.h CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150997/new/ https://reviews.llvm.org/D150997 Files: clang-tools-extra/include-cleaner/incl

[PATCH] D150997: [llvm] Split out DenseMapInfo specialization

nikic added a comment. The pre-merge checks have a build failure in clang-tools-extra. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150997/new/ https://reviews.llvm.org/D150997 ___ cfe-commits mailing l

[PATCH] D150997: [llvm] Split out DenseMapInfo specialization

IncludeGuardian added inline comments. Comment at: llvm/include/llvm/ADT/DenseMapInfo.h:321 - static bool isEqual(const Variant &LHS, const Variant &RHS) { -return LHS == RHS; - } Regardless of this change, I think the equality here needs to be fixed in the

[PATCH] D150997: [llvm] Split out DenseMapInfo specialization

IncludeGuardian updated this revision to Diff 524024. IncludeGuardian added a comment. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits. Fix one missing `` include in tools and add `DenseMapInfoVariant.h`, which I forgot previously Repository: rG LLVM Github M