<html dir="ltr">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1">
<style id="owaParaStyle">P {
MARGIN-TOP: 0px; MARGIN-BOTTOM: 0px
}
</style>
</head>
<body fPStyle="1" ocsi="0">
<div style="direction: ltr;font-family: Tahoma;color: #000000;font-size: 10pt;">
<p>> I think this is going to be OK. I've got a couple comments though:</p>
<div style="FONT-FAMILY: Times New Roman; COLOR: #000000; FONT-SIZE: 16px">
<div>
<div dir="ltr">
<div>><br>
</div>
<div>
<div>> - layout.getFieldOffset(fieldNo), tunit, RecordTy);</div>
<div>> + layout.getFieldOffset(fieldNo - 1), tunit, RecordTy);</div>
<div>><br>
</div>
</div>
<div>
<div>> 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.</div>
<div> </div>
<div>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.</div>
<div>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).</div>
<div> </div>
<div>> b) Instead of inlining all the code for CollectRecordStaticVars how about outlining it into a couple of static functions?</div>
<div> </div>
<div>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.)</div>
<div> </div>
<div>> c) Will this handle something like this:</div>
<div>><br>
</div>
<div>> class A {</div>
<div>> static int a;</div>
<div>> };</div>
<div>><br>
</div>
<div>> A a;</div>
<div>><br>
</div>
<div>> where the variable A::a isn't defined in the file (but likely is somewhere else).</div>
<div> </div>
<div>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.</div>
<div> </div>
<div>--paulr</div>
<div> </div>
</div>
</div>
</div>
</div>
</div>
</body>
</html>