[PATCH] D51597: [ASTImporter] Fix import of VarDecl init

[Aleksei Sidorin via Phabricator via cfe-commits]

a_sidorin added a comment.

Hi Gabor,
The change looks mostly fine but the difference with ASTReader approach disturbs me a bit.



================
Comment at: lib/AST/ASTImporter.cpp:1441
+  To->setInit(ToInit);
+  if (From->isInitKnownICE()) {
+    EvaluatedStmt *Eval = To->ensureEvaluatedStmt();
----------------
I see that this is only a code move but I realized that ASTReader and ASTImporter handle this case differently. ASTReader code says:

```
    if (Val > 1) { // IsInitKnownICE = 1, IsInitNotICE = 2, IsInitICE = 3
      EvaluatedStmt *Eval = VD->ensureEvaluatedStmt();
      Eval->CheckedICE = true;
      Eval->IsICE = Val == 3;
    }
```
but ASTimporter sets these bits only if `isInitKnownICE()` is `true`. This looks strange.


================
Comment at: lib/AST/ASTImporter.cpp:3190
+            // The VarDecl in the "From" context has a definition, but in the
+            // "To" context we already has a definition.
+            VarDecl *FoundDef = FoundVar->getDefinition();
----------------
have (same below)


================
Comment at: unittests/AST/ASTImporterTest.cpp:3312
+  ASSERT_NE(FromDWithInit, FromDWithDef);
+  ASSERT_EQ(FromDWithDef->getPreviousDecl() ,FromDWithInit);
+
----------------
Formatting of comma is broken. Same below.


Repository:
  rC Clang

https://reviews.llvm.org/D51597