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

[Aleksei Sidorin via Phabricator via cfe-commits]

a.sidorin added a comment.

Hello Peter,

Looks mostly good, but there are some minor comments.



================
Comment at: lib/AST/ASTImporter.cpp:5500
+
+  TemplateArgumentListInfo ToTAInfo;
+  TemplateArgumentListInfo *ResInfo = nullptr;
----------------
xazax.hun wrote:
> szepet wrote:
> > xazax.hun wrote:
> > > According to phabricator this code is very similar to a snippet starting from line 4524 and some code bellow. Maybe it would be worth to have a function instead of duplicating?
> > Good point, I would do it in a separate patch and add it as a dependency if it is OK.
> Sure, that would be awesome. :)
This is a template I have prepared for a patch still non-commited yet:

```
    template<typename InContainerTy>
    bool ImportTemplateArgumentListInfo(const InContainerTy &Container,
                                        TemplateArgumentListInfo &ToTAInfo) {
      for (const auto &FromLoc : Container) {
        if (auto ToLoc = ImportTemplateArgumentLoc(FromLoc))
          ToTAInfo.addArgument(*ToLoc);
        else
          return true;
      }
      return false;
    }
```


================
Comment at: lib/AST/ASTImporter.cpp:5626
+  DeclarationName Name = Importer.Import(E->getName());
+  if(!E->getName().isEmpty() && Name.isEmpty())
+    return nullptr;
----------------
Needed space after if.


================
Comment at: unittests/AST/ASTImporterTest.cpp:574
+                         "template <typename T> void declToImport() {"
+                         "S<T>::foo;"
+                         "}",
----------------
Could you please align the code as conventions require?


https://reviews.llvm.org/D38845