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:
> > > 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.
> Oh, so we're adding those to the *end* of the translation unit, not at the
> beginning?
We are adding them at the beginning, but it seems we import the other stuff
before adding them, so on the import tests they show up on the end.
The thing here is that we end up importing them, but then adding new ones from
the current Sema anyway. But this is fine as most other import things, they get
separate declarations but merged in any case so they have the same canonical
decl.
The way we avoid doing that for the other synthesized builtins around it, is
that we just skip creating them if we find their `Identifier` has been created
somehow, which is a fairly strange way to test this.
This seemed even less appropriate for `va_list_tag`, since some ABIs put it
into `std` namespace.
And it seems to me that avoiding creating them again for the current Sema is a
fairly minimal optimization, and it could make we miss diagnosing imports where
those things are inconsistent between contexts.
WDYT?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D136886/new/
https://reviews.llvm.org/D136886
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
