<div dir="ltr">Some more stylistic comments:<div><br></div><div>The description that you have on CheckShadowInheritedVariables isn't really the type of comments that we have in doxygen form. Im not sure if its in line with the rest of the code.</div><div><br></div><div>The ignore warning comments are restating what is in the code, please remove them.</div><div><br></div><div>Could you make the header and the source file match the name?</div><div><br></div><div>Why are you checking that the DeclContext has a definition rather than the record itself?</div><div><br></div><div>Space after the <<.</div><div><br></div><div>Don't use the cast for the check, use isa. Although, since you use the value later, it is probably better to write this as:</div><div><br></div><div> if (const auto *RD = cast<CXXRecordDecl>(CurContext))</div><div> CheckShadowInheritedVariabless(Loc, Name.getAsString(), RD, RD);</div><div><br></div><div><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Jan 24, 2017 at 4:06 PM, James Sun via cfe-commits <span dir="ltr"><<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div bgcolor="white" lang="EN-US" link="#0563C1" vlink="#954F72">
<div class="m_-4331983902763705655WordSection1">
<p class="MsoNormal"><span style="font-size:11.0pt">Coding style change<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt"><u></u> <u></u></span></p>
<div style="border:none;border-top:solid #b5c4df 1.0pt;padding:3.0pt 0in 0in 0in">
<p class="MsoNormal"><b><span style="color:black">From: </span></b><span style="color:black">James Sun <<a href="mailto:jamessun@fb.com" target="_blank">jamessun@fb.com</a>><br>
<b>Date: </b>Tuesday, January 24, 2017 at 2:36 PM<br>
<b>To: </b>"<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>" <<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>><br>
<b>Subject: </b>Add warning for c++ member variable shadowing<u></u><u></u></span></p>
</div><div><div class="h5">
<div>
<p class="MsoNormal"><span style="font-family:"Times New Roman""><u></u> <u></u></span></p>
</div>
<p class="MsoNormal"><span style="font-size:11.0pt">Dear members</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt"> </span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt">Here is a patch (attached) to create warnings where a member variable shadows the one in one of its inheriting classes. For cases where we really don't want to shadow member variables, e.g.
</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt"> </span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt">class a {</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt"> int foo; </span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt">}</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt"> </span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt">class b : a { </span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt"> int foo; // Generate a warning</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt">}</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt"> </span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt">This patch </span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt">(1) adds a member variable shadowing checking, and
</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt">(2) incorporates it to the unit tests.</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt"> </span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt"> </span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt">Comments are welcome.</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt"> </span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt">Thanks</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt"> </span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt">James</span><u></u><u></u></p>
</div></div></div>
</div>
<br>______________________________<wbr>_________________<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" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/cfe-commits</a><br>
<br></blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="gmail_signature" data-smartmail="gmail_signature">Saleem Abdulrasool<br>compnerd (at) compnerd (dot) org</div>
</div>