BRevzin added inline comments.
================
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:
> BRevzin wrote:
> > 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.
> Is this true for all iterators? Or some quirk of how this one is written/used
> (that could be fixed/changed there instead)?
So I undid this change to copy the exact issue that I ran into. But it actually
ended up still compiling anyway. Part of the issue might be that I keep futzing
with the cmake configuration since it takes me more than an hour to compile, so
maybe there's some target that needed this change that I no longer compile.
But the kind of problem I think this had was:
```
template <typename T>
struct iterator {
T* p;
template <typename U>
iterator(iterator<U> rhs)
: p(rhs.p)
{ }
bool operator==(iterator const& rhs);
};
bool check(iterator<int const> a, iterator<int> b) {
return a == b;
}
```
which compiles fine in C++17 but is ambiguous in C++20 because
`b.operator==(a)` is also a candidate (even though it's not _really_ a
candidate, and would be a hard error if selected). the sfinae removes the bad
candidate from the set.
It's true for all iterators in general in that you want `const_iterator`
constructible from `iterator` but not the reverse (unless they're the same
type).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D78938/new/
https://reviews.llvm.org/D78938
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
