<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<meta name="Generator" content="Microsoft Word 14 (filtered medium)">
<style><!--
/* Font Definitions */
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
@font-face
{font-family:Tahoma;
panose-1:2 11 6 4 3 5 4 4 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0in;
margin-bottom:.0001pt;
font-size:12.0pt;
font-family:"Times New Roman","serif";}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:blue;
text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
{mso-style-priority:99;
color:purple;
text-decoration:underline;}
span.im
{mso-style-name:im;}
span.EmailStyle18
{mso-style-type:personal-reply;
font-family:"Calibri","sans-serif";
color:#1F497D;}
.MsoChpDefault
{mso-style-type:export-only;
font-family:"Calibri","sans-serif";}
@page WordSection1
{size:8.5in 11.0in;
margin:1.0in 1.0in 1.0in 1.0in;}
div.WordSection1
{page:WordSection1;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]-->
</head>
<body lang="EN-US" link="blue" vlink="purple">
<div class="WordSection1">
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">About to be away for a week, but I will take this up (and the other one) when I get back.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">--paulr<o:p></o:p></span></p>
<p class="MsoNormal"><a name="_MailEndCompose"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></a></p>
<div style="border:none;border-left:solid blue 1.5pt;padding:0in 0in 0in 4.0pt">
<div>
<div style="border:none;border-top:solid #B5C4DF 1.0pt;padding:3.0pt 0in 0in 0in">
<p class="MsoNormal"><b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif"">From:</span></b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif""> David Blaikie [mailto:dblaikie@gmail.com]
<br>
<b>Sent:</b> Tuesday, May 17, 2016 3:05 PM<br>
<b>To:</b> reviews+D19567+public+1ee0c82c0824bedd@reviews.llvm.org; David Blaikie<br>
<b>Cc:</b> Robinson, Paul; Aaron Ballman; Adrian Prantl; cfe-commits<br>
<b>Subject:</b> Re: [PATCH] D19567: PR21823: 'nodebug' attribute on global/static variables<o:p></o:p></span></p>
</div>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<p class="MsoNormal">ping<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<p class="MsoNormal">On Mon, May 2, 2016 at 11:51 AM, David Blaikie via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>> wrote:<o:p></o:p></p>
<p class="MsoNormal" style="margin-bottom:12.0pt">dblaikie added a comment.<br>
<br>
In <a href="http://reviews.llvm.org/D19567#414906" target="_blank">http://reviews.llvm.org/D19567#414906</a>, @probinson wrote:<br>
<br>
> Huh. There are strange interactions here, which makes me even more nervous about testing fewer cases.<br>
<br>
<br>
Generally this sort of thing makes me more interested in testing fewer cases so we can see/make sure they're properly covering, as you just did by the sounds of it. It's hard to see if everything's really covered if there's lots of redundant coverage that adds
noise to the test case.<br>
<br>
> As it happens, the test as written did not exercise all 3 modified paths. Because 'struct S2' had all its members marked with nodebug, none of the static-member references caused debug info for the class as a whole to be attempted.<br>
<br>
<br>
Not sure I quite follow. Even without nodebug:<br>
<br>
struct foo { static const int x = 3; int y; };<br>
int i = foo::x;<br>
<br>
doesn't produce any debug info for 'foo', x, etc. Just for 'i'.<br>
<br>
> I needed to add a variable of type S2 (without 'nodebug') in order to exercise that path. Once that was done, then the modification to CollectRecordFields became necessary.<br>
<br>
> Even in an unmodified compiler, "static_const_member" never shows up as a DIGlobalVariable, although the other cases all do. So, testing only for DIGlobalVariable wouldn't be sufficient to show that it gets suppressed by 'nodebug'.<br>
<br>
<br>
Have you tested cases where the static member is ODR used and/or defined:<br>
<br>
struct foo { static const int x = 3; };<br>
const int foo::x; // defined<br>
void sink(void*);<br>
void use() { sink(&foo::x); } // ODR used<br>
<br>
I'm guessing that the out of line definition will create the DIGlobalVariable you're not seeing. But probably through a common codepath you've already corrected for.<br>
<br>
The ODR use probably doesn't cause anything interesting to happen.<br>
<br>
> "static_member" shows up unless we have modified both EmitGlobalVariable(VarDecl case) and CollectRecordFields, given that the test actually tries to emit debug info for S2 as a whole.<br>
<br>
<br>
I would imagine this could still boil down to: check-not DIGlobalVariable, check-not DIFlagStaticMember ?<br>
<br>
But once the test is smaller/more targeted, checking the extra details of the member list of a composite type, etc, seems OK.<br>
<br>
<span class="im">> So, the genuinely most-minimal did-this-change-do-anything test would need only "static_member" and "const_global_int_def" to show that each path had some effect. This approach does not fill me with warm fuzzies, or the feeling that future
changes in this area will not break the intended behavior. What do you think?</span><br>
<br>
<span class="im">> --paulr</span><br>
<br>
<br>
<o:p></o:p></p>
<div>
<div>
<p class="MsoNormal">Repository:<br>
rL LLVM<br>
<br>
<a href="http://reviews.llvm.org/D19567" target="_blank">http://reviews.llvm.org/D19567</a><br>
<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><o:p></o:p></p>
</div>
</div>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
</div>
</div>
</body>
</html>