[PATCH] [clang-tidy] Assert related checkers

Szabolcs Sipos szabolcs.sipos at ericsson.com
Mon Feb 16 05:28:59 PST 2015


================
Comment at: clang-tidy/misc/AssertSideEffectCheck.cpp:46-53
@@ +45,10 @@
+    if (OpCallExpr) {
+      OverloadedOperatorKind OpKind = OpCallExpr->getOperator();
+      return OpKind == OO_Equal || OpKind == OO_PlusEqual ||
+             OpKind == OO_MinusEqual || OpKind == OO_StarEqual ||
+             OpKind == OO_SlashEqual || OpKind == OO_SlashEqual ||
+             OpKind == OO_PipeEqual || OpKind == OO_CaretEqual ||
+             OpKind == OO_LessLessEqual || OpKind == OO_GreaterGreaterEqual ||
+             OpKind == OO_PlusPlus || OpKind == OO_MinusMinus ||
+             OpKind == OO_PercentEqual;
+    }
----------------
LegalizeAdulthood wrote:
> What about `OO_AmpEqual`?
That's what I wanted to write instead of the second OO_SlashEqual.

================
Comment at: clang-tidy/misc/StaticAssertCheck.cpp:90
@@ +89,3 @@
+    if (AssertExprRoot) {
+      AssertExprRootHint = FixItHint::CreateRemoval(
+          SourceRange(AssertExprRoot->getOperatorLoc()));
----------------
alexfh wrote:
> I think, in case there's a message, the fixes should be made as follows:
>   * insert a comma right after the end of the LHS (matters if there is a line break or a comment after the LHS)
>   * remove the "&&" and any whitespace after it
>   * don't touch the message at all.
> 
> In case there is no message originally, just insert a ", \"\"" right before the closing brace.
> 
> WDYT?
I also thought about this solution but it would generate incorrect source code if the argument of assert was in parentheses or "&&" had its operands swapped. These cases might be rare but my main aim was not to generate incorrect code.

About fixing the 'comma problem' while keeping the genericness:
I could have ignored the second case or written an even bigger matcher (and FixIt).
In the first case, distinguishing between parentheses in the argument or in the macro definition and deciding where the remaining whitespaces belong would make the code so complicated that I thought it didn't worth it.

If you suggest generating a nice-looking source code and risking that it will not always complie, I will change that part.

http://reviews.llvm.org/D7375

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the cfe-commits mailing list