<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:x="urn:schemas-microsoft-com:office:excel" 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:"Cambria Math";
panose-1:2 4 5 3 5 4 6 3 2 4;}
@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:0cm;
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;}
p.MsoAcetate, li.MsoAcetate, div.MsoAcetate
{mso-style-priority:99;
mso-style-link:"Balloon Text Char";
margin:0cm;
margin-bottom:.0001pt;
font-size:8.0pt;
font-family:"Tahoma","sans-serif";}
span.xapple-converted-space
{mso-style-name:x_apple-converted-space;}
span.EmailStyle18
{mso-style-type:personal-reply;
font-family:"Calibri","sans-serif";
color:#1F497D;}
span.BalloonTextChar
{mso-style-name:"Balloon Text Char";
mso-style-priority:99;
mso-style-link:"Balloon Text";
font-family:"Tahoma","sans-serif";}
.MsoChpDefault
{mso-style-type:export-only;
font-size:10.0pt;}
@page WordSection1
{size:612.0pt 792.0pt;
margin:70.85pt 70.85pt 70.85pt 70.85pt;}
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="SV" link="blue" vlink="purple">
<div class="WordSection1">
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">Thanks for the comments. New iteration with the comments addressed.<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">+ + static PathDiagnosticLocation createConditionalColonLoc(<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">+ + const ConditionalOperator *CO,<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">+ + const SourceManager &SM);<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">+ + I think the new style is to either break after the return type or to double-indent wrapped arguments, rather than right-aligning + them. (Right-aligning
is harder to maintain.)<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">I'd like to keep this as it is since there are other create functions next to it that uses this style, this also applies to the ccp file. We could
make a separate patch later and change them all to the new style. I believe the coding standard also states that we should use the same style already used in the source.<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">//Anders<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<div>
<div style="border:none;border-top:solid #B5C4DF 1.0pt;padding:3.0pt 0cm 0cm 0cm">
<p class="MsoNormal"><b><span lang="EN-US" style="font-size:10.0pt;font-family:"Tahoma","sans-serif"">From:</span></b><span lang="EN-US" style="font-size:10.0pt;font-family:"Tahoma","sans-serif""> Jordan Rose [mailto:jordan_rose@apple.com]
<br>
<b>Sent:</b> den 4 december 2013 18:29<br>
<b>To:</b> Anders Rönnholm<br>
<b>Cc:</b> cfe-commits@cs.uiuc.edu; Per Viberg; Daniel Marjamäki<br>
<b>Subject:</b> Re: [PATCH] new check checking use of identical expressions inside a conditional expression, i.e. '?'.<o:p></o:p></span></p>
</div>
</div>
<p class="MsoNormal"><span lang="EN-US"><o:p> </o:p></span></p>
<div>
<div>
<p class="MsoNormal">Looking good! But I still think we should just highlight the true and false expressions, not the entire conditional (which may span multiple lines):<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<div>
<p class="MsoNormal">+ BR.EmitBasicReport(<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">+ AC->getDecl(), "Identical expressions in conditional expression",<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">+ categories::LogicError,<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">+ "identical expressions on both sides of ':' in conditional expression",<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">+ ELoc,C->getSourceRange());<o:p></o:p></p>
</div>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">You can simply construct a SourceRange[] on the stack and then pass that to the ArrayRef<SourceRange> argument.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">More comments:<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<div>
<p class="MsoNormal">+ static PathDiagnosticLocation createConditionalColonLoc(<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">+ const ConditionalOperator *CO,<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">+ const SourceManager &SM);<o:p></o:p></p>
</div>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">I think the new style is to either break after the return type or to double-indent wrapped arguments, rather than right-aligning them. (Right-aligning is harder to maintain.)<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<div>
<p class="MsoNormal"> static bool isIdenticalExpr(const ASTContext &Ctx, const Expr *Expr1,<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">- const Expr *Expr2);<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">+ const Expr *Expr2,<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">+ const bool IgnoreSideEffects = false);<o:p></o:p></p>
</div>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">We generally don't bother making parameters const. Pointers-to-const, yes, but not the parameters themselves. (It does help catch stupid mistakes sometimes, but it's not worth being inconsistent with the rest of Clang.)<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<div>
<p class="MsoNormal">+ if (isIdenticalExpr(AC->getASTContext(), C->getTrueExpr(),<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">+ C->getFalseExpr(),true)) {<o:p></o:p></p>
</div>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">Missing space after the comma.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<div>
<p class="MsoNormal">- if (!Child1 || !Child2 || !isIdenticalExpr(Ctx, Child1, Child2))<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">+ if (!Child1 || !Child2 || !isIdenticalExpr(Ctx, Child1, Child2,<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">+ IgnoreSideEffects))<o:p></o:p></p>
</div>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">Indentation is wrong; the 'IgnoreSideEffects' is an argument to 'isIdenticalExpr', so it should be aligned with the first argument in the call. (I misread this at first as another || possibility.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<div>
<p class="MsoNormal"> case Stmt::ArraySubscriptExprClass:<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">+ case Stmt::CallExprClass:<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"> case Stmt::CStyleCastExprClass:<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"> case Stmt::ImplicitCastExprClass:<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"> case Stmt::ParenExprClass:<o:p></o:p></p>
</div>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">Aren't calls only identical if IgnoreSideEffects is true? (Or if the callee is constexpr or __attribute__((pure)) or ((const)), I suppose, but I'm okay with leaving that out.)<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">You could argue that calls being identical on both sides of an operator might still be an error because the calls are unsequenced, but I suppose that doesn't matter if you're using == or !=. So I don't think we can treat them as identical
here, much as I'd like to, unless we have IgnoreSideEffects.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">Please add tests for the comparisons operators involving calls as well.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<div>
<p class="MsoNormal"> PathDiagnosticLocation<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">+ PathDiagnosticLocation::createConditionalColonLoc(<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">+ const ConditionalOperator *CO,<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">+ const SourceManager &SM) {<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">+ return PathDiagnosticLocation(CO->getColonLoc(), SM, SingleLocK);<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">+}<o:p></o:p></p>
</div>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">Similar to above: prefer double-indented parameters to right-aligned parameters, and no need for that first indent before the qualified name. (I see other functions in the file are doing that, but not new ones.)<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<div>
<p class="MsoNormal">+void test_expr_negative_func() {<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">+ unsigned a = 0;<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">+ unsigned b = 1;<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">+ a = a > 5 ? a+func() : a+func2(); // no warning<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">+}<o:p></o:p></p>
</div>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">Please also include a test where you're calling the same function with different arguments, to demonstrate that we're stepping into the CallExpr.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">Thanks!<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">Jordan<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<div>
<p class="MsoNormal">On Nov 27, 2013, at 4:43 , Anders Rönnholm <<a href="mailto:Anders.Ronnholm@evidente.se">Anders.Ronnholm@evidente.se</a>> wrote:<o:p></o:p></p>
</div>
<p class="MsoNormal"><br>
<br>
<o:p></o:p></p>
<div>
<div>
<div>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif"">Hi,</span><span style="font-size:11.0pt;font-family:"Calibri","sans-serif""><o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif""> </span><span style="font-size:11.0pt;font-family:"Calibri","sans-serif""><o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif"">I have updated the check to warn on a++,a=1 and a+func() as the comments suggested.</span><span style="font-size:11.0pt;font-family:"Calibri","sans-serif""><o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif""> </span><span style="font-size:11.0pt;font-family:"Calibri","sans-serif""><o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif"">/Anders</span><span style="font-size:11.0pt;font-family:"Calibri","sans-serif""><o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif""> </span><span style="font-size:11.0pt;font-family:"Calibri","sans-serif""><o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif"">> I think we should actually be warning, since only one side of the branch will ever be executing. Maybe we can include a flag here that says whether the expression
walker should step into calls? What do you think?</span><span style="font-size:11.0pt;font-family:"Calibri","sans-serif""><o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif""> </span><span style="font-size:11.0pt;font-family:"Calibri","sans-serif""><o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif"">This sounds reasonable. We could warn about other "non-const" expressions too, I believe:</span><span style="font-size:11.0pt;font-family:"Calibri","sans-serif""><o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif""> </span><span style="font-size:11.0pt;font-family:"Calibri","sans-serif""><o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif""> a ? b++ : b++;</span><span style="font-size:11.0pt;font-family:"Calibri","sans-serif""><o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif""> a ? b=2 : b=2;</span><span style="font-size:11.0pt;font-family:"Calibri","sans-serif""><o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif""> </span><span style="font-size:11.0pt;font-family:"Calibri","sans-serif""><o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif"">Best regards,</span><span style="font-size:11.0pt;font-family:"Calibri","sans-serif""><o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif"">Daniel Marjamäki</span><span style="font-size:11.0pt;font-family:"Calibri","sans-serif""><o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif""> </span><span style="font-size:11.0pt;font-family:"Calibri","sans-serif""><o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif"">..................................................................................................................</span><span style="font-size:11.0pt;font-family:"Calibri","sans-serif""><o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif"">Daniel Marjamäki Senior Engineer</span><span style="font-size:11.0pt;font-family:"Calibri","sans-serif""><o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif"">Evidente ES East AB Warfvinges väg 34 SE-112 51 Stockholm Sweden</span><span style="font-size:11.0pt;font-family:"Calibri","sans-serif""><o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif""> </span><span style="font-size:11.0pt;font-family:"Calibri","sans-serif""><o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif"">Mobile: +46 (0)709 12 42 62</span><span style="font-size:11.0pt;font-family:"Calibri","sans-serif""><o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif"">E-mail: Daniel.Marjamaki at<span class="xapple-converted-space"> </span><a href="http://evidente.se/"><span style="color:purple">evidente.se</span></a></span><span style="font-size:11.0pt;font-family:"Calibri","sans-serif""><o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif""> </span><span style="font-size:11.0pt;font-family:"Calibri","sans-serif""><o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif""><a href="http://www.evidente.se/"><span style="color:purple">www.evidente.se</span></a><o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif""> <o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif"">________________________________________<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif"">Från: Jordan Rose [jordan_rose at<span class="xapple-converted-space"> </span><a href="http://apple.com/"><span style="color:purple">apple.com</span></a>]<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif"">Skickat: den 20 november 2013 18:22<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif"">Till: Per Viberg</span><span style="font-size:11.0pt;font-family:"Calibri","sans-serif""><o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif"">Cc: cfe-commits at<span class="xapple-converted-space"> </span><a href="http://cs.uiuc.edu/"><span style="color:purple">cs.uiuc.edu</span></a>; Daniel Marjamäki</span><span style="font-size:11.0pt;font-family:"Calibri","sans-serif""><o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif"">Ämne: Re: [PATCH] new check checking use of identical expressions inside a conditional expression, i.e. '?'.</span><span style="font-size:11.0pt;font-family:"Calibri","sans-serif""><o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif""> </span><span style="font-size:11.0pt;font-family:"Calibri","sans-serif""><o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif"">Hm, interesting. In this case:</span><span style="font-size:11.0pt;font-family:"Calibri","sans-serif""><o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif""> </span><span style="font-size:11.0pt;font-family:"Calibri","sans-serif""><o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif"">+void test_expr_negative_func() {</span><span style="font-size:11.0pt;font-family:"Calibri","sans-serif""><o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif"">+ unsigned a = 0;</span><span style="font-size:11.0pt;font-family:"Calibri","sans-serif""><o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif"">+ unsigned b = 1;</span><span style="font-size:11.0pt;font-family:"Calibri","sans-serif""><o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif"">+ a = a > 5 ? a+func() : a+func(); // no warning</span><span style="font-size:11.0pt;font-family:"Calibri","sans-serif""><o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif"">+}</span><span style="font-size:11.0pt;font-family:"Calibri","sans-serif""><o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif""> </span><span style="font-size:11.0pt;font-family:"Calibri","sans-serif""><o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif"">I think we should actually be warning, since only one side of the branch will ever be executing. Maybe we can include a flag here that says whether the expression
walker should step into calls? What do you think?</span><span style="font-size:11.0pt;font-family:"Calibri","sans-serif""><o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif""> </span><span style="font-size:11.0pt;font-family:"Calibri","sans-serif""><o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif"">Other than that, this looks good, except that I would add the true-expr and false-expr as ranges to highlight in addition to putting the warning itself on the
colon.</span><span style="font-size:11.0pt;font-family:"Calibri","sans-serif""><o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif""> </span><span style="font-size:11.0pt;font-family:"Calibri","sans-serif""><o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif"">Jordan<o:p></o:p></span></p>
</div>
</div>
</div>
</div>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
</div>
</body>
</html>