[PATCH] D78938: Make LLVM build in C++20 mode

[Barry Revzin via Phabricator via llvm-commits]

BRevzin added inline comments.


================
Comment at: llvm/include/llvm/DebugInfo/DWARF/DWARFExpression.h:167-171
-
-inline bool operator!=(const DWARFExpression::iterator &LHS,
-                       const DWARFExpression::iterator &RHS) {
-  return !(LHS == RHS);
-}
----------------
dblaikie wrote:
> Why are some being removed? That seems harder to justify. Even if they're not called, it may be more valuable to have the symmetry to reduce friction if/when they are needed. (iterators seem pretty common to compare for inequality - such as in a loop condition testing I != E)
They're not being removed. These functions still exist - it's just that now they're being injected by the base class template with this exact signature (rather than before where they were slightly different), so that now these are redefinition issues. 

There's no loss of functionality here. 


================
Comment at: llvm/include/llvm/IR/BasicBlock.h:324-325
+    template <typename PHINodeU, typename BBIteratorU,
+              typename = std::enable_if_t<
+                  std::is_convertible<PHINodeU *, PHINodeT *>::value>>
     phi_iterator_impl(const phi_iterator_impl<PHINodeU, BBIteratorU> &Arg)
----------------
dblaikie wrote:
> What tripped over/required this SFINAE?
There's somewhere which compared a const iterator to a non-const iterator, that ends up doing conversions in both directions under C++20 rules, one direction of which is perfectly fine and the other was a hard error. Need to make the non-const iterator not constructible from a const iterator.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78938