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

[Gabor Marton via Phabricator via cfe-commits]

martong marked 9 inline comments as done.
martong added inline comments.


================
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();
----------------
a_sidorin wrote:
>  ASTImporter is not very const-friendly, but this function is pure, so I think it's better to const-qualify the parameters.
Ok, added the const qualifiers.


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


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


================
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.
----------------
a_sidorin wrote:
> That's interesting. Have you tried '-frewrite-includes' for expanding inclusions only without macro expansion?
I hadn't tried it before, but I've just done that now. Unfortunately, the AST produced with `-E -frewrite-includes` produces a type for the operator() that already has a canonical type: `long (long) const`

```
) ./bin/clang-check -p cxx_cmd_json ~w/llvm3/git/llvm-project/libcxx/src/filesystem/directory_iterator.cpp -ast-dump -ast-dump-filter "posix_utimes" | ag "CXXMethodDecl.*int_type"                                           --- COMMAND ---
  |           | |-CXXMethodDecl 0x3007ec0 <col:30, line:400:3> line:396:18 used operator() 'int_type (long) const' inline
  |           | |-CXXMethodDecl 0x30131f0 <col:18, line:400:3> line:396:18 implicit __invoke 'int_type (long)' static inline
  |           | |-CXXMethodDecl 0x2ddfaf0 <col:30, line:400:3> line:396:18 used operator() 'int_type (long) const' inline
  |           | |-CXXMethodDecl 0x2deae20 <col:18, line:400:3> line:396:18 implicit __invoke 'int_type (long)' static inline
(venv) egbomrt at elx78373d5s|~/WORK/llvm3/build/release_assert
) ./bin/clang-check -p cxx_cmd_json ~w/llvm3/git/llvm-project/libcxx/src/filesystem/directory_iterator.cpp -ast-dump -ast-dump-filter "posix_utimes" | ag "CXXMethodDecl.*operator"
  |           | |-CXXMethodDecl 0x2833020 <col:30, line:400:3> line:396:18 used operator() 'int_type (long) const' inline
  |           | |-CXXMethodDecl 0x287fa20 <col:30, line:400:3> line:396:18 used operator() 'int_type (long) const' inline
(venv) egbomrt at elx78373d5s|~/WORK/llvm3/build/release_assert
) ./bin/clang -x c++ directory_iterator.cpp.E.orig -fsyntax-only -Xclang -ast-dump -Xclang -ast-dump-filter -Xclang posix_utimes | ag "CXXMethodDecl.*operator"
  |           | |-CXXMethodDecl 0x31c6ad0 <col:30, line:400:3> line:396:18 used operator() 'long (long) const' inline
(venv) egbomrt at elx78373d5s|~/WORK/llvm3/build/release_assert
) ./bin/clang -x c++ directory_iterator.cpp.E.ri -fsyntax-only -Xclang -ast-dump -Xclang -ast-dump-filter -Xclang posix_utimes | ag "CXXMethodDecl.*operator"
  |           | |-CXXMethodDecl 0x25ad7f0 <col:30, line:400:3> line:396:18 used operator() 'long (long) const' inline
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70819