[PATCH] D131685: [clang][AST] RecursiveASTVisitor should visit owned TagDecl of friend type.

[David Rector via Phabricator via cfe-commits]

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