[PATCH] D70819: [ASTImporter] Support functions with placeholder return types ...

[Aleksei Sidorin via Phabricator via cfe-commits]

a_sidorin added a comment.

Hello Gabor,
That's an interesting case, thank for fixing this!



================
Comment at: clang/lib/AST/ASTImporter.cpp:3008
+// which is equal to the given DC.
+bool isAncestorDeclContextOf(DeclContext *DC, Decl *D) {
+  DeclContext *DCi = D->getDeclContext();
----------------
 ASTImporter is not very const-friendly, but this function is pure, so I think it's better to const-qualify the parameters.


================
Comment at: clang/lib/AST/ASTImporter.cpp:3020
+  QualType FromTy = D->getType();
+  const FunctionProtoType *FromFPT = FromTy->getAs<FunctionProtoType>();
+  if (AutoType *AutoT = FromFPT->getReturnType()->getContainedAutoType()) {
----------------
Is it possible for getAs() to return nullptr at this point?


================
Comment at: clang/lib/AST/ASTImporter.cpp:3174
+  bool UsedDifferentProtoType = false;
+  const auto *FromFPT = FromTy->getAs<FunctionProtoType>();
+  if (FromFPT) {
----------------
If FromFPT is not used outside of the condition, we can move the initialization inside if().


================
Comment at: clang/unittests/AST/ASTImporterTest.cpp:5609
+      R"(
+      auto X = [](long l){
+        using int_type = long;
----------------
Nit: a space before '{'.


================
Comment at: clang/unittests/AST/ASTImporterTest.cpp:5625
+  // parsed libcxx/src/filesystem/directory_iterator.cpp, but could not reduce
+  // that with creduce, because after preprocessing, the AST no longer
+  // contained the TypeAlias as a return type of the lambda.
----------------
That's interesting. Have you tried '-frewrite-includes' for expanding inclusions only without macro expansion?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70819