teemperor accepted this revision.
teemperor added a comment.
This revision is now accepted and ready to land.

I think the test could be made a bit stricter (see inline comment), but 
otherwise this LGTM. Thanks!



================
Comment at: clang/unittests/AST/ASTImporterTest.cpp:5900
+  ASSERT_TRUE(ToD);
+  EXPECT_EQ(FromD->isCopyDeductionCandidate(), 
ToD->isCopyDeductionCandidate());
+  // Check that the deduced class template is also imported.
----------------
Not sure if that's actually testing what it should. `FromD` is not the copy 
deduction candidate, so this is just comparing that both Decls have the default 
value `false`?

If we pick the copy deduction candidate in this test then we could test that we 
actually copy the value over and not just have the default 'false' flag for 
`isCopyDeductionCandidate` (something like 
`cxxDeductionGuideDecl(hasParameter(0, hasType(asString("A<T>"))))` should find 
the copy deduction candidate).

Then we could also simplify this to 
`EXPECT_TRUE(ToD->isCopyDeductionCandidate());`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92109

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to