[llvm] [ADT] Make null PointerUnion with different active members compare equal (PR #121847)

Sergei Barannikov via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 7 07:02:02 PST 2025


s-barannikov wrote:

> Hm, I'm not really convinced this change is right. If PointerUnion allows having null pointers of different types, then treating them as equal may not be correct in general.

I think I can agree with that. This makes sense if you think of `PointerUnion` as `std::variant`, which compares unequal if the compared objects have different active members.

> I think the fix here should either be that assigning null pointer that is not nullptr_t to a PointerUnion is invalid,

The assignment happens [here](https://github.com/llvm/llvm-project/blob/1849244685bc42b07b1b14e3f62e15c535e74c39/llvm/lib/CodeGen/MachineRegisterInfo.cpp#L198). It does not call an assignment operator, it constructs a `PointerUnion` from the specified (typed) null pointer and then calls copy constructor. If we want to prohibit this, we need to add a dynamic check to the constructor accepting a typed pointer. I can reporpose this PR to do that.

> or that dyn_cast_if_present should account for the different null pointers.

I tried to specialize `ValueIsPresent` for `PointerUnion`, but either this is not possible or I don't know how to do that properly. Whatever I tried resulted in "ambiguous partial specialization" errors. Here is the generic specialization for nullable types:
https://github.com/llvm/llvm-project/blob/2a4992e9e34de6b74aa8c254fea867d1271b3e97/llvm/include/llvm/Support/Casting.h#L622

which needs to be changed to something like:
```C++
  static inline bool isPresent(const T &t) { return static_cast<bool>(t); }
```


https://github.com/llvm/llvm-project/pull/121847


More information about the llvm-commits mailing list