[PATCH] Do not split variables definitions into 2 DIEs.

[David Blaikie]

On Tue, Oct 21, 2014 at 2:01 PM, Frédéric Riss <friss at apple.com> wrote:

>
> On Oct 20, 2014, at 2:06 PM, David Blaikie <dblaikie at gmail.com> wrote:
>
> I've checked this out on the GDB test suite & it seems fine.
>
> Looking at the code itself, I think it could be made a bit more
> obvious/uniform to have VariableDIE always refer to the definition and have
> just a single conditional to decide whether to attach the
> DW_AT_specification or not.
>
> Actually this works pretty well & even adds another source fidelity
> benefit, allowing the definition, while out-of-line of the class, to be
> attributed to the scope in which the definition occurs (eg: namespace x {
> struct foo { static int i; }; int foo::i; } - putting the definition in
> namespace 'x' instead of just forcing it out to the global namespace) and
> exposes a bug in Clang where the definition is scoped to the decl context
> instead of the lexical context. (patch attached to show a slightly
> short-sighted fix there)
>
>
> Yeah, this looks an even nicer cleanup. I re-tested it just to be sure.
> Can you just commit what’s in your branch, or do you want me to reroll your
> reworked patch with a proper commit message?
>

It'll need the clang change first, or we'll end up putting the static
variable's definition inside the class (DW_AT_class_type with both a
DW_AT_member and a DW_AT_variable as children, both describing the static
member variable, with the latter pointing to the former via
DW_AT_specification) & I'll need to fix/work up test coverage for it (so if
you want to do those things instead, I won't complain - but I can do them).

What was your thinking on the scoping issue in the frontend? It's certainly
not the full solution - the full solution, imho would be to emit a
declaration (potentially the DW_AT_member in the class) whenever the
lexical context and the decl context differ. This would be the precise
source fidelity. (& in the backend, do the right thing for fields that
differ between the two - so we can describe the location of the definition
in the .cpp file as different from the location of the declaration in the
.h file, etc)

 - David


> Thanks!
>

Thanks for your patience,

- David


> Fred
>
> If we did a few more smart things in the frontend, we could get better
> fidelity for declarations/definitions of namespace/global variables -
> removing the weird special case introduced with my patch. Probably the
> right thing here is to optimize in the frontend, if the definition appears
> in the decl context (eg: if the global variable's decl context is the same
> as the lexical context) then just produce the definition. If the decl and
> lexical context differ, produce both a declaration (in the decl context)
> and a definition (in the lexical context) - this would be the best source
> fidelity, but probably isn't too worthwhile.
>
> Oh, and if we wanted to be really good here, we could make sure we emit
> any attributes onto the definition that differ from the declaration - note
> GCC does the right thing here, putting DW_AT_decl_line/file on the
> definitions if the line/file happens to be different from the declaration's
> line/file. We don't do that - we just put nothing on the definition (& I
> don't think we do this for function declarations/definitions either (again,
> here, GCC gets this right)... could, but probably not important)
>
>
>
> On Thu, Oct 16, 2014 at 8:46 AM, Frederic Riss <friss at apple.com> wrote:
>
>> ping
>>
>> http://reviews.llvm.org/D5457
>>
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>
> <static_var_def.diff><global_var.diff>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141021/93340c55/attachment.html>