[PATCH] D105354: [clang][AST] Add support for DecompositionDecl to ASTImporter.

[Gabor Marton via Phabricator via cfe-commits]

martong accepted this revision.
martong added inline comments.
This revision is now accepted and ready to land.


================
Comment at: clang/lib/AST/ASTImporter.cpp:2305-2309
+  BindingDecl *ToD;
+  if (GetImportedOrCreateDecl(ToD, D, Importer.getToContext(), DC, Loc,
+                              Name.getAsIdentifierInfo()))
+    return ToD;
+
----------------
balazske wrote:
> shafik wrote:
> > martong wrote:
> > > So, we moved the import of the binding before importing the decomposition decl to avoid an infinite recursion. But why can't we have an infinit recursion this way?
> > > 
> > > Perhaps, it would be useful to have a test case that triggered the infinity in case of the original order of the import calls.
> > Yes, I agree, I would also like to understand better why this avoids the infinite recursion problem, a test case would be helpful as well as an explanation of the steps that leads us to the problem.
> With the import at original place, in `VisitVarDecl` the bindings (which are `BindingDecl`) are imported before create of the `DecompositionDecl` instance, and in `VisitBindingDecl` the decomposition (a `DecompositionDecl` that is `VarDecl`) is imported before create of the `BindingDecl` instance. This causes the recursion with the most simple import of a `DecompositionDecl`. This is triggered by the existing simple test (actually I discovered the problem at the test failure). If the decomposition is imported after create of the `BindingDecl` the same `BindingDecl` is not imported again because it already exists.
Ok, thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105354