[PATCH] D4784: [clang-tidy] Add check for possibly incomplete switch statements
Piotr Zegar via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Sun Jul 16 06:34:41 PDT 2023
PiotrZSL 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);
----------------
xgupta wrote:
> 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.
Just add
```
std::optional<TraversalKind> getCheckTraversalKind() const override {
return TK_IgnoreUnlessSpelledInSource;
}
```
to header, like it is in other checks.
this should also solve template handling, so add test like this:
```
template<typename T>
void testTemplate(T value)
{
switch(value) {
...
}
}
testTemplate(5);
testTemplate(SomeEnum);
```
================
Comment at: clang-tools-extra/docs/clang-tidy/checks/list.rst:95
`bugprone-inaccurate-erase <bugprone/inaccurate-erase.html>`_, "Yes"
+ `bugprone-switch-missing-default-case <bugprone/switch-missing-default-case.html>`_, "Yes"
`bugprone-incorrect-roundings <bugprone/incorrect-roundings.html>`_,
----------------
check does not provide fixes, so remove that "Yes"
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D4784/new/
https://reviews.llvm.org/D4784
More information about the cfe-commits
mailing list