berkaysahiin wrote:
> That new matcher _is_ much more robust, however I think we can still come up
> with an example that breaks it:
>
> ```diff
> switch (1) {
> case 1:
> case 2:
> if (a == 1)
> f(0);
> - else if (a == 2)
> + else case 3: if (a == 2)
> return;
> else // <-- false positive
> f(1);
> }
> ```
>
> That code is pretty evil, but I feel... if we _can_ handle it, we should. And
> I do think we can, but that will need a new matcher, something like:
>
> ```c++
> AST_MATCHER_P(Stmt, ignoringLabelsAndCaseStatements,
> ast_matchers::internal::Matcher<Stmt>, InnerMatcher) {
> for (const Stmt *S = &Node;;) {
> if (const auto *Case = dyn_cast<SwitchCase>(S))
> S = Case->getSubStmt();
> else if (const auto *Label = dyn_cast<LabelStmt>(S))
> S = Label->getSubStmt();
> else
> return S && InnerMatcher.matches(*S, Finder, Builder);
> }
> }
> ```
>
> Which can be used like:
>
> ```c++
> Finder->addMatcher(compoundStmt(forEach(ignoringLabelsAndCaseStatements(
> IfWithInterruptingThenElse)))
> .bind("cs"),
> this);
> ```
>
> An upside of the custom matcher is that it should also handle other edge
> cases like:
>
> ```c++
> // Mixed labels and case statements:
> switch (a) {
> case 1:
> case 2:
> label:
> if (0)
> return;
> else // <-- 'else' should be removed
> f(0);
> }
>
> // Stacked labels:
> label1:
> label2:
> if (0)
> return;
> else // <-- 'else' should be removed
> f(0);
> ```
Thanks a lot for the detailed review. Will spend more time on this one
https://github.com/llvm/llvm-project/pull/181878
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
