[Lldb-commits] [PATCH] D55571: [ast] CreateParameterDeclaration should use an appropriate DeclContext.

[Zachary Turner via Phabricator via lldb-commits]

zturner added a comment.

In D55571#1330354 <https://reviews.llvm.org/D55571#1330354>, @friss wrote:

> Zachary, how did you figure out this can be an issue? Does it fix something we should be testing?


Recently I added a `target modules dump ast` command and I've been using it to write tests against (see lldb/lit/SymbolFile/NativePDB for an example of what some of these tests look like).  While experimenting with this command, I noticed that I had some ParmVarDecls under the translation unit decl (e.g. global scope), which obviously is something that isn't possible.  I only tested this with the PDB parser, so I can't confirm whether the same situation could occur int he DWARF parser, but since this is strictly building the AST *after* we've parsed the debug info, I definitely think it would be a problem for DWARF.

So the million dollar question is: What happens when you have function parameters in the AST at global scope?  I think it falls into "undefined behavior" territory - i.e. there's no way to know what clang will do, since you're not supposed to have that situation.

As for as testing, I've found this command extremely useful for writing ast reconstruction tests -- Do a few things in the debugger, run the command, FileCheck the output.  I think "did we construct a valid AST from the debug info?" is a largely unexplored testing surface in LLDB, so this definitely opens up a lot of possibilities for testing on this front.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D55571