apple
Avoid calling `Node::deepEquals` if one of the node pointers is null.
This is a companion to https://github.com/swiftlang/swift/pull/78812 (but can be merged independently first). I'm trying to fix a subtle bug in the tests where null pointers were silently being let through, which traps in certain environments with more strict pointer enforcement.
In that change, Node::deepEquals now asserts if either of the arguments is null. The rationale is that the caller should be responsible for what happens in this situation; i.e., is a null pointer a demangling failure that should proceed no further, or an expected "no node" state? These LLDB tests appear to treat null as the latter situation, so I've updated them to state that explicitly.
@adrian-prantl Does this look reasonable to you? I was surprised to see nulls getting into this function but perhaps it's intentional based on which types are being tested.
Friendly ping: @adrian-prantl @JDevlieghere any concerns with this change?
Accidentally stumbled across this PR, but I'd suggest considering moving NullSafeNodeDeepEquals to lldb/source/Plugins/TypeSystem/Swift/SwiftDemangle.h. It'll likely be useful in other places (TypeSystemSwiftTypeRef, SwiftLanguageRuntimeDynamicTypeResolution, etc).
Also, as a side note, I understand the point you've made in https://github.com/swiftlang/swift/pull/78812 about NodePointer being a currency type. Still, at the same time, I'm afraid that this will not be the last case of accidental use of this function with potentially NULL arguments. It'd be nice if the compiler enforced null-safety, or we could at least provide a safe overload as a hint to a programmer to be cautious.
