[PATCH] D38845: [ASTImporter] Support importing UnresolvedMemberExpr, DependentNameType, DependentScopeDeclRefExpr

[Peter Szecsi via Phabricator via cfe-commits]

szepet marked an inline comment as done.
szepet added inline comments.


================
Comment at: unittests/AST/ASTImporterTest.cpp:1569
+        FirstDeclMatcher<FunctionTemplateDecl>().match(*TB, Pattern);
+    if (!FromDSDRE)
+      return;
----------------
martong wrote:
> I think, this `if` would be needed only if the test suite would be parameterized also with `ArgVector{"-fdelayed-template-parsing"}`, but that is not.
Its necessary, since on windows platforms we have that flag by default (so, it does not matter that we dont include here, it will be used anyway).


================
Comment at: unittests/AST/ASTImporterTest.cpp:1573
+    auto To = cast<FunctionTemplateDecl>(Import(FromDSDRE, Lang_CXX));
+    EXPECT_TRUE(FirstDeclMatcher<FunctionTemplateDecl>().match(To, Pattern));
+  }
----------------
martong wrote:
> It will be hard to notice based on the test output which DSDRE's import was unsuccessful. Therefore I'd unroll the for loop instead, also please see the above comment that it would be more practical to have one FromTU.
I understand the concern, but I am really against the mentioned design. The main point of this code is that whenever we would like to add one more test cases, then we need to add it to the vector and everything else is done. Other than that, I would say that debugging a broken test case is already a "time expensive" job, so in that case it wouldnt make much of a different, since you need to look at the code closely anyway.


https://reviews.llvm.org/D38845