davrec added inline comments.
================ Comment at: clang/unittests/AST/ASTContextParentMapTest.cpp:145 + EXPECT_THAT(Ctx.getParents(FrBLoc), ElementsAre(DynTypedNode::create(FrB))); + if (FrATagDecl) + EXPECT_THAT(Ctx.getParents(*FrATagDecl), ---------------- sammccall wrote: > I'm confused why this is conditional: isn't this the main thing that we're > testing? Why do we want the test to silently pass if the AST structure > changes so that ownedTagDecl is null? It's hard to tell from reading the code > if anything is being tested at all. Good point, agree we should expect FrATagDecl to be nonnull. And we should check that FrBTagDecl is null, or at a minimum that FrBTagDecl != FrATagDecl, because if they are the same decl we are traversing it twice. (This could happen if someone decides to try to reuse the ElaboratedType of A's friend decl in B's, to save memory or whatever.) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131685/new/ https://reviews.llvm.org/D131685 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
