dodohand added inline comments.
================ Comment at: clang-tools-extra/clang-tidy/misc/AssignmentInIfClauseCheck.cpp:39 + diag(MatchedDecl->getBeginLoc(), + "Assignment detected within if statement. Fix to equality check if this " + "was accidental. Consider moving out of if statement if intentional."); ---------------- gribozavr2 wrote: > Please follow Clang's message style. The message should start with a > lowercase letter and should not end with a period. Suggested fixes (since > there are multiple) should be in notes. > > warning: an assignment within an 'if' condition is bug-prone > note: if it should be an assignment, move it out of the 'if' condition > note: if it is meant to be an equality check, change '=' to '==' > > Also consider adding a fixit to the second note. Will adjust the messages. I'm specifically reluctant to add a fixit to this, as I'm not sure how I can tell an intended assignment (for which the fix would be moving the assignment out of the if clause) apart from an accidental assignment (for which the fix is changing '=' to '=='). Thoughts? ================ Comment at: clang-tools-extra/docs/clang-tidy/checks/misc-assignment-in-if-clause.rst:3 + +misc-assignment-in-if-clause +============================ ---------------- gribozavr2 wrote: > WDYT about `bugprone-assignment-in-if-condition`? I'm in favor of that categorization - it reflects how I feel about having assignments within an 'if' condition. I'll change it accordingly Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127114/new/ https://reviews.llvm.org/D127114 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
