[PATCH] D32751: [ASTImporter] Support new kinds of declarations (mostly Using*)

[Aleksei Sidorin via Phabricator via cfe-commits]

a.sidorin added inline comments.


================
Comment at: lib/AST/ASTImporter.cpp:2993
+      return nullptr;
+  }
+
----------------
szepet wrote:
> nit: As I see these cases typically handled in the way:
> 
> ```
> FrPattern = .;
> ToPattern = ..;
> if(FrPattern && !ToPattern)
> ```
> Just to avoid the nested ifstmt.
The logic is a bit more complicated. There are 3 cases:
# Both `FromPattern` and `ToPattern` are `nullptr`s. Just continue.
#  `FromPattern` is non-null and `ToPattern` is null. Return error  (`nullptr`).
# Both `FromPattern` and `ToPattern` are `nullptr`s. Do the `set...` action.

So, it will require nested `if`s or a code like:
```
if (FromPattern && ToPattern)
  set...
if (FromPattern && !ToPattern)
  return nullptr;
```



================
Comment at: lib/AST/ASTImporter.cpp:3000
+    else
+      // FIXME: We return a nullptr here but the definition is already created
+      // and available with lookups. How to fix this?..
----------------
szepet wrote:
> I dont see the problem with moving these up , collect nad investigate them in a smallset before the Create function, then adding them to the created ToUsing decl. It could be done as a follow up patch, dont want to mark it as a blocking issue.
There is a chicken and egg problem: both UsingShadowDecl and UsingDecl reference each other and UsingShadowDecl gets referenced UsingDecl as a ctor argument. If you have a good idea on how to resolve this dependency correctly, please point me.


https://reviews.llvm.org/D32751