[cfe-commits] [PATCH] PR14471: Debug info for static data members (Clang part)

[Robinson, Paul]

> I think this is going to be OK. I've got a couple comments though:

>
> -                          layout.getFieldOffset(fieldNo), tunit, RecordTy);
> +                          layout.getFieldOffset(fieldNo - 1), tunit, RecordTy);
>
> a) There are a few other places that use fieldNo and since you're not updating them it'd be nice to have some comments.

I see 4 uses total. One increments it, one decrements it, and this reference uses the value; it had to become "fieldNo - 1" because the increment isn't inside the "for" statement anymore.  Those uses seem like normal tracking behavior and don't warrant any extra commentary.  If you still want some, let me know.
The 4th use is in the lambda part of the method where AFAICT it will always be zero (and possibly should be "fieldno" not "fieldNo", is that a bug? I know squat about lambdas).

> b) Instead of inlining all the code for CollectRecordStaticVars how about outlining it into a couple of static functions?

Hmm, yeah the body of that loop is basically "if static-member do this; else if field do that;" so factoring out those bits seems reasonable.  (The lambda part, which I didn't touch, probably also could be factored out for similar reasons, but that might exceed the scope of what I'm doing with this patch.  Let me know.)

> c) Will this handle something like this:
>
> class A {
>   static int a;
> };
>
> A a;
>
> where the variable A::a isn't defined in the file (but likely is somewhere else).

Sure. You'll get a DIE inside the class with DW_AT_external and DW_AT_declaration, and no DW_AT_location; and then a global variable DIE for the global "a" with type A.  llvm-dwarfdump fragment attached.

--paulr

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130103/d329e142/attachment.html>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: Static-member-example.txt
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130103/d329e142/attachment.txt>