<html><head></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><br><div><div>On Dec 29, 2010, at 2:05 PM, Chandler Carruth wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite">This adds a warning to Clang for self-assignment of the form:<div><br></div><blockquote class="gmail_quote" style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex; ">
int a = 42;<br>a = a;</blockquote><div><br></div><div>Code with this often is merely trying to mark a variable used, but there is a more idiomatic way of doing that:</div><div><br></div><blockquote class="gmail_quote" style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex; ">
(void)a;</blockquote><div><br></div><div>The warning is mostly a stylistic issue to help catch unintended self-assignments which result from typos in a codebase which strictly enforces that it shouldn't be used intentionally. As such, it is ignored by default, but can be turned on with -Wself-assign.</div></blockquote><div><br></div><div>--- a/include/clang/Basic/DiagnosticGroups.td</div><div>+++ b/include/clang/Basic/DiagnosticGroups.td</div><div>@@ -95,6 +95,7 @@ def : DiagGroup<"pointer-to-int-cast">;</div><div> def : DiagGroup<"redundant-decls">;</div><div> def ReturnType : DiagGroup<"return-type">;</div><div> def BindToTemporaryCopy : DiagGroup<"bind-to-temporary-copy">;</div><div>+def SelfAssignment : DiagGroup<"self-assign">;</div><div> def SemiBeforeMethodBody : DiagGroup<"semicolon-before-method-body">;</div><div> def : DiagGroup<"sequence-point">;</div><div> def Shadow : DiagGroup<"shadow">;</div><div><br></div><div>Turn this on with -Wall, perhaps? I'd eventually like to have this on by default, but starting with -Wall seems best.</div><div><br></div><div><div>--- a/lib/Sema/SemaExpr.cpp</div><div>+++ b/lib/Sema/SemaExpr.cpp</div><div>@@ -7458,6 +7458,33 @@ static inline UnaryOperatorKind ConvertTokenKindToUnaryOpcode(</div><div> return Opc;</div><div> }</div><div> </div><div>+/// DiagnoseSelfAssignment - Emits a warning if a value is assigned to itself.</div><div>+/// This warning is only emitted for builtin assgnment operations. It is also</div><div>+/// suppressed in the event of macro expansions.</div><div><br></div><div>Typo "assgnment".</div><div><br></div><div>+static void DiagnoseSelfAssignment(Sema &S, Expr *lhs, Expr *rhs,</div><div>+ SourceLocation OpLoc) {</div><div>+ if (!S.ActiveTemplateInstantiations.empty())</div><div>+ return;</div><div>+ if (OpLoc.isInvalid() || OpLoc.isMacroID())</div><div>+ return;</div><div>+ lhs = lhs->IgnoreParenImpCasts();</div><div>+ rhs = rhs->IgnoreParenImpCasts();</div><div>+ const DeclRefExpr *LeftDeclRef = dyn_cast<DeclRefExpr>(lhs);</div><div>+ const DeclRefExpr *RightDeclRef = dyn_cast<DeclRefExpr>(rhs);</div><div>+ if (!LeftDeclRef || !RightDeclRef ||</div><div>+ LeftDeclRef->getLocation().isMacroID() ||</div><div>+ RightDeclRef->getLocation().isMacroID())</div><div>+ return;</div><div>+ const Decl *LeftDecl = LeftDeclRef->getDecl()->getCanonicalDecl();</div><div>+ const Decl *RightDecl = RightDeclRef->getDecl()->getCanonicalDecl();</div><div>+ if (LeftDecl != RightDecl)</div><div>+ return;</div><div><br></div><div>Hrm, should you check whether the type of the declaration is a volatile (or a reference to volatile)?</div><div><br></div><div><br></div><div>+ S.Diag(OpLoc, diag::warn_self_assignment)</div><div>+ << LeftDeclRef->getType()</div><div>+ << lhs->getSourceRange() << rhs->getSourceRange();</div><div>+}</div><div>+</div><div><br></div><div>It would be really nice if we could follow up this warning with a note + Fix-It, e.g.,</div><div><br></div><div> note: cast 'thevariablename' to 'void' to silence this warning</div><div><br></div><div>with a Fix-It that replaces the expression with</div><div><br></div><div> (void)thevariablename</div><div><br></div><div>Perhaps we could add this note as a follow-on when an expression statement consists of just a self-assignment?</div><div><br></div><div><span class="Apple-tab-span" style="white-space:pre"> </span>- Doug</div></div><div><br></div><div><br></div></div></body></html>