<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Sep 23, 2014 at 10:06 AM, Frederic Riss <span dir="ltr"><<a href="mailto:friss@apple.com" target="_blank">friss@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">================<br>
Comment at: test/DebugInfo/X86/gnu-public-names.ll:56<br>
@@ -56,2 +55,3 @@<br>
+; CHECK: [[STATIC_MEM_VAR:0x[0-9a-f]+]]: DW_TAG_member<br>
 ; CHECK-NEXT: DW_AT_name {{.*}} "static_member_variable"<br>
<br>
----------------<br>
</span><span class="">dblaikie wrote:<br>
> This actually seems like it /might/ be a reasonable place for a separate declaration and definition, since the user wrote an out of line definition.<br>
><br>
> What location do we attribute the definition to? It might be worth checking the decl_line attribute in this test. Did we previously attribute the declaration and definition to their respective lines, or just duplicate one/the other of them?<br>
><br>
> How does this compare to other nested definitions (I suppose for functions in namespaces we don't emit both distinguishing the declaration and definition in cases like: "namespace x { void func(); } void x::func() {}" so there's arguably no reason to similarly for variables)<br>
</span>I would agree totally if we somehow emitted information corresponding to the definition location in the debug info. But the only line information that we provide is the one of the declaration in all cases (the attribute is called decl_line, so maybe that's really how it should be). If the definition DIE has no overlapping attributes with the declaration, I don't see a reason to keep both.<br>
There is one thing that is special to class members though. If we only emit a definition (what the patch actually does), it will be inside the type. And this is be an issue if we want to emit type units, isn't it?</blockquote><div><br></div><div>It could be, but I think it'll actually be fine. The trick we pull is that members that aren't definitively part of types (implicit special members, nested types, member template specializations, and, I believe, static member definitions) aren't actually added to the types member list - they're just given the type as their context reference. This means when we emit the type for a type unit, and we just walk the member list, we don't get any of these things that can vary per translation unit, we just get the pristine type.<br><br>So I /believe/ that putting the definition of a static variable inside the type is not actually a problem (and furthermore, I believe we could even do this for member function definitions under some circumstances*)<br><br>If this makes the code any simpler - I'd be happy for it to go in, but please provide a small test case showing that this does the right thing for type units (& for non-type units, of course). I believe it will.<br><br>* This would work fine in type units, but in the absence of type units we could end up getting both a declaration and a definition of the member. So then we might end up needing to either emit /just/ the definition from the frontend and have the backend know how to split that into a declaration and definition (so it could put the declaration in the type unit and the definition in the compile_unit itself) or have the frontend be type unit-aware... hmm, no, we could just make the backend smarter and have it only out-of-line the definition when not in a type unit.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> Maybe this whole split-if-not-file/function-scope heuristic is to only ever emit declarations into classes.<br>
For functions in namespaces, we only emit one definition DIE, but for class methods we split declaration and definition. So maybe we should really keep the class type exempt of definitions.<br>
<br>
<a href="http://reviews.llvm.org/D5457" target="_blank">http://reviews.llvm.org/D5457</a><br>
<br>
<br>
</blockquote></div><br></div></div>