xgupta marked 3 inline comments as done.
xgupta added inline comments.
================
Comment at:
clang-tools-extra/clang-tidy/bugprone/SwitchMissingDefaultCaseCheck.cpp:16-29
+void SwitchMissingDefaultCaseCheck::registerMatchers(
+ ast_matchers::MatchFinder *Finder) {
+ Finder->addMatcher(
+ switchStmt(has(implicitCastExpr().bind("cast")),
+ unless(hasAncestor(switchStmt(has(defaultStmt())))))
+ .bind("switch"),
+ this);
----------------
PiotrZSL wrote:
> I do not like that implicitCastExpr.
>
> this is AST for your example:
> ```
> `-SwitchStmt <line:4:3, line:7:3>
> |-ImplicitCastExpr <line:4:11> 'int' <LValueToRValue>
> | `-DeclRefExpr <col:11> 'int' lvalue Var 0x555de93bc200 'i' 'int'
> `-CompoundStmt <col:14, line:7:3>
> `-CaseStmt <line:5:3, line:6:5>
> |-ConstantExpr <line:5:8> 'int'
> | |-value: Int 0
> | `-IntegerLiteral <col:8> 'int' 0
> `-BreakStmt <line:6:5>
> ```
>
> look that case there is only because we got cast from rvalue to lvalue.
> if you write example like this:
>
> ```
> int getValue();
>
> void Positive() {
> switch (getValue()) {
> case 0:
> break;
> }
> }
>
> ```
>
> there is not cast because AST will look like this:
> ```
> -SwitchStmt <line:4:3, line:7:3>
> |-CallExpr <line:4:11, col:20> 'int'
> | `-ImplicitCastExpr <col:11> 'int (*)()' <FunctionToPointerDecay>
> | `-DeclRefExpr <col:11> 'int ()' lvalue Function 0x5573a3b38100
> 'getValue' 'int ()'
> `-CompoundStmt <col:23, line:7:3>
> `-CaseStmt <line:5:3, line:6:5>
> |-ConstantExpr <line:5:8> 'int'
> | |-value: Int 0
> | `-IntegerLiteral <col:8> 'int' 0
> `-BreakStmt <line:6:5>
> ```
>
> So add test with rvalue int... remove that implicit cast checks, and use
> IgnoreUnlessSpelledInSource (it will remove IntegralCast cast from enum to
> int, so hasCondition would match enum directly)
Removed the implicit cast but couldn't get how to use
IgnoreUnlessSpelledInSource here.
================
Comment at:
clang-tools-extra/clang-tidy/bugprone/SwitchMissingDefaultCaseCheck.cpp:19
+ Finder->addMatcher(
+ switchStmt(has(implicitCastExpr().bind("cast")),
+ unless(hasAncestor(switchStmt(has(defaultStmt())))))
----------------
PiotrZSL wrote:
> this should be something like:
> ```hasCondition(expr(hasType(qualType(hasCanonicalType(unless(hasDeclaration(enumDecl()))))))```
> Or you can verify just if type is integral type.
> Note that with modern C++ you may have init statements in enums.
>
For some reason, the check is giving warning for enum cases and I couldn't
understand why, can you please help?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D4784/new/
https://reviews.llvm.org/D4784
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
