[PATCH] D73675: Avoid many std::tie/tuple instantiations in ASTImporter

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

Thanks for this patch too!
However, I have a minor concern with this new approach:
If `importSeq` fails then a `ToSomething` still holds a value from the "From" context. This could cause problems if it is used later in creating the new AST node. Normally, this should not happen because we do check the return value of `importSeq`. But as the AST increases, new properties are added to a node, let's say `ToSomething2` and when we forget (by mistake) to add this to `importSeq` then we end up having a node that has a reference to something in the "From" context:

  auto ToSomething = From->getToSomething();
  auto ToSomething2 = From->getToSomething2();
  if (Error E = ImportSeq(ToSomething)) 
    return std::move(E);
  createNewNodeInToCtx(ToSomething, ToSomething2);

In contrast to the original behavior where we would end up with an uninitialized value (pointer) which seems to be easier to recognize when someone uses the merged AST.
(I am not sure how could we force `ToSomething2` into `importSeq`, perhaps a clang-tidy checker could do that.)



================
Comment at: clang/lib/AST/ASTImporter.cpp:1265
+
+  if (Error E = importSeq(ToEPI.ExceptionSpec.NoexceptExpr,
+                           ToEPI.ExceptionSpec.SourceDecl,
----------------
My concern here is that if `importSeq` fails here then `ToEPI.ExceptionSpec.NoexceptExpr` still holds a value from the "From" context and that can be a (fatal) problem later. The import should happen right after the initialization of these variables.
So, we should have something like this:
```
  ToEPI.ExceptionSpec.NoexceptExpr = FromEPI.ExceptionSpec.NoexceptExpr;
  ToEPI.ExceptionSpec.SourceDecl = FromEPI.ExceptionSpec.SourceDecl;
  ToEPI.ExceptionSpec.SourceTemplate = FromEPI.ExceptionSpec.SourceTemplate;
  if (Error E = importSeq(ToEPI.ExceptionSpec.NoexceptExpr,ToEPI.ExceptionSpec.SourceTemplate))
    return std::move(E);
  ToEPI.ExtInfo = FromEPI.ExtInfo;
  ToEPI.Variadic = FromEPI.Variadic;
  ToEPI.HasTrailingReturn = FromEPI.HasTrailingReturn;
  ToEPI.TypeQuals = FromEPI.TypeQuals;
  ToEPI.RefQualifier = FromEPI.RefQualifier;
  ToEPI.ExceptionSpec.Type = FromEPI.ExceptionSpec.Type;
  ToEPI.ExceptionSpec.Exceptions = ExceptionTypes;
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73675