[PATCH] D110751: [DomTree] Assert that blocks in queries aren't from another function

[Fangrui Song via Phabricator via llvm-commits]

MaskRay accepted this revision.
MaskRay added a comment.

LGTM.



================
Comment at: llvm/include/llvm/Support/GenericDomTree.h:353
+    assert((!BB || !BB->getParent() || BB->getParent() == Parent) &&
+           "A node from another function!");
+
----------------
DaniilSuchkov wrote:
> aeubanks wrote:
> > DaniilSuchkov wrote:
> > > nikic wrote:
> > > > Is `!BB` really a valid input to this function?
> > > I don't know, but since there are no checks for that anywhere and it's not mentioned in its description, I decided refrain from making any assumptions.
> > > If you think it's not, I can add an assertion for that in a separate patch.
> > I'd just remove the check for `!BB` if check-llvm doesn't complain
> `make check` quickly reminded me that `!BB ||` wasn't accidental at all:
> ```
>   Skipped          :    36
>   Unsupported      : 19985
>   Passed           : 19299
>   Expectedly Failed:    58
>   Failed           :  6325
> ```
> That's almost 1/3 of all tests.
If my memory doesn't fail me: it may be used by postdominator as the root node. 
I did notice that this should be avoided to prevent confusion.


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

https://reviews.llvm.org/D110751