<html><head></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><br><div><div>On Jul 22, 2011, at 11:41 AM, Anna Zaks wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; ">Attached are 3 patches that add more support for adding */& mismatch fixits. In particular, C function parameters would get handled as well as assignments in general.<div><br></div><div>1) In the first patch, I refactored most of the fixit generation functionality out of SemaOverload and provided a simple conversion checking function. This patch also includes a bug fix - the range for FixItHint::CreateRemoval was not specified correctly.</div><div></div></div><span><deref_addof_refactor.diff></span></blockquote><div><br></div><div><div>+ typedef bool (*TypeComparisonFuncTy) (const CanQualType FromTy,</div><div>+ const CanQualType ToTy,</div><div>+ Sema &S,</div><div>+ SourceLocation Loc,</div><div>+ ExprValueKind VK);</div><div><br></div><div>Could you name 'VK' something like 'FromVK', so we know which type the value kind refers to?</div><div><br></div></div><div><div>+ bool NeedParen = true;</div><div>+ if (isa<ParenExpr>(FullExpr) ||</div><div>+ isa<DeclRefExpr>(Expr) ||</div><div>+ isa<ArraySubscriptExpr>(Expr) ||</div><div>+ isa<CallExpr>(Expr) ||</div><div>+ isa<MemberExpr>(Expr))</div><div>+ NeedParen = false;</div><div>+ if (const UnaryOperator *UO = dyn_cast<UnaryOperator>(Expr))</div><div>+ if (UO->isPostfix())</div><div>+ NeedParen = false;</div><div><br></div><div>I'd prefer if you did an audit of the various expression kinds we have in Clang, so that you can white-list all of the ones that don't need to be parenthesized here. Otherwise, we're setting ourselves up for a long tail of QoI bugs where Clang suggests parentheses unnecessarily. (We don't need test cases for every expression kind noted here, of course).</div><div><br></div><div><div>Index: lib/Sema/SemaFixItUtils.cpp</div><div>===================================================================</div><div>--- lib/Sema/SemaFixItUtils.cpp<span class="Apple-tab-span" style="white-space:pre"> </span>(revision 0)</div><div>+++ lib/Sema/SemaFixItUtils.cpp<span class="Apple-tab-span" style="white-space:pre"> </span>(revision 0)</div><div>@@ -0,0 +1,145 @@</div><div>+//===--- SemaFixItUtils.cpp - Sema FixIts ---------------------------------===//</div></div><div><br></div><div>Please add this .cpp file to lib/Sema/CMakeLists.txt.</div><div><br></div><div>With those minor tweaks, please go ahead and commit this patch!</div></div><br><blockquote type="cite"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div>2) This one adds fixit generation to the Sema::<span class="Apple-style-span" style="font-family: Monaco; font-size: 11px; ">DiagnoseAssignmentResult().</span></div><div></div></div><span><deref_addof_assign.diff></span></blockquote><div><br></div></div><div><blockquote type="cite"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div>3) Test the fixits generated as a result of patch #2.</div><div></div></div><span><deref_addof_assign_tests.diff></span><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div></div></div></blockquote></div><div><br></div><div>These two look great, thanks!</div><div><br><div><span class="Apple-tab-span" style="white-space:pre"> </span>- Doug</div></div></body></html>