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

[Frédéric Riss]

> On Oct 21, 2014, at 2:09 PM, David Blaikie <dblaikie at gmail.com> wrote:
> 
> 
> 
> On Tue, Oct 21, 2014 at 2:01 PM, Frédéric Riss <friss at apple.com <mailto:friss at apple.com>> wrote:
> 
>> On Oct 20, 2014, at 2:06 PM, David Blaikie <dblaikie at gmail.com <mailto: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).

Oh that’s what it does, sorry, I thought you were just talking about file/line attribution of the definition… Interestingly it doesn’t seem to bother lldb much, but it’s obviously too ugly to live! The file/line issue is of course related. I thought it was the backend grabbing the wrong information from the Decls, but it’s in fact the frontend passing the wrong information down.

> 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)

Well, you have sent out the wrong patch for the frontend part, thus I misunderstood the exact issue you were trying to solve. If you send the right patch I can try to make sense of it and get an opinion on the matter.

Fred

>  - 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 <mailto:friss at apple.com>> wrote:
>> ping
>> 
>> http://reviews.llvm.org/D5457 <http://reviews.llvm.org/D5457>
>> 
>> 
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu <mailto:llvm-commits at cs.uiuc.edu>
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits <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/4045c6ac/attachment.html>