[PATCH] D136886: [clang] ASTImporter: Fix importing of va_list types and declarations

Mon Oct 31 06:18:05 PDT 2022

mizvekov added inline comments.

================
Comment at: clang/test/AST/ast-dump-overloaded-operators.cpp:27
 // CHECK-NEXT: | `-ParmVarDecl {{.*}} <col:18> col:19{{( imported)?}} 'E'
-// CHECK-NEXT: `-FunctionDecl {{.*}} <line:14:1, line:18:1> line:14:6{{( imported)?}} test 'void ()'
+// CHECK-NEXT:  -FunctionDecl {{.*}} <line:14:1, line:18:1> line:14:6{{( imported)?}} test 'void ()'
 // CHECK-NEXT:   `-CompoundStmt {{.*}} <col:13, line:18:1>
----------------
aaron.ballman wrote:
> mizvekov wrote:
> > aaron.ballman wrote:
> > > This looks like a benign typo -- we still match the line because FileCheck will match partial lines, but I'm pretty sure nothing in your patch would have necessitated this change. Then again, you make this change in a lot of tests, so maybe I'm wrong -- in which case, what changed?
> > What is happening here (and in all the other such instances) is that on the `import` case, this declaration stops being the last one on the TU. So the beginning of the line would match on `|-` instead of ``-`, but the non-import case this remains the last one.
> > 
> > So I simply relaxed the match.
> Hmmm, I think it'd help to show what new lines are now showing up so we can validate that they make sense in context. WDYT?
It's the new lines from the synthesized `va_list_tag`. I think they would be noise on this these tests, they are testing something completely unrelated.

But they do show up on the ast-json tests where we are basically dumping the whole TU.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136886