[PATCH] D92109: [ASTImporter] Support import of CXXDeductionGuideDecl

Thu Nov 26 06:59:20 PST 2020

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