[PATCH] D151079: [llvm][ADT] Change isEqual for DenseMapInfo<std::variant<...>>

[Kadir Cetinkaya via Phabricator via llvm-commits]

kadircet abandoned this revision.
kadircet added a subscriber: sammccall.
kadircet added inline comments.


================
Comment at: llvm/include/llvm/ADT/DenseMapInfo.h:326-329
+    template <typename T, typename U>
+    bool operator()(const T &LHS, const U &RHS) const {
+      return false;
+    }
----------------
IncludeGuardian wrote:
> As this will be instantiated for each combination of types in our variant, I think the approach taken in https://reviews.llvm.org/D151079 is slightly better for compilation speed, although it is not as simple as this.
> 
> I would love to suggest C-style variadic functions to avoid any template instantiation, i.e.
> 
> ```
>     bool operator()(...) const {
>       return false;
>     }
> ```
> 
> but I don't think the behavior is guaranteed for non-POD user-defined types and some compilers will create a copy.
yes that was my concern as well, hence we had an offline discussion with @sammccall and he was kind enough to come up with go/phab/D151557. so i'll abandon this patch in favor of it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151079