Avoid calling Node::deepEquals if one of the node pointers is null.
#10201
Conversation
This is a companion to swiftlang/swift#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 Also, as a side note, I understand the point you've made in swiftlang/swift#78812 about |
This is a companion to swiftlang/swift#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::deepEqualsnow 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.