lebedev.ri added inline comments.
================
Comment at:
clang-tools-extra/clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp:367-368
+ bool TraverseStmt(Stmt *Node) {
+ if (!Node)
+ return Base::TraverseStmt(Node);
+
----------------
aaron.ballman wrote:
> If there's not a node, do we really need to traverse it?
This is consistent across this whole `FunctionASTVisitor`, and is consistent
with other `RecursiveASTVisitor<>`s,
so i'll leave this as-is. If this is wrong, other places should be changed too.
================
Comment at:
clang-tools-extra/clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp:381
+ // FIXME: "each method in a recursion cycle" Increment is not implemented.
+ case Stmt::ConditionalOperatorClass:
+ case Stmt::SwitchStmtClass:
----------------
aaron.ballman wrote:
> `BinaryConditionalOperatorClass` as well (for all the places we're dealing
> with `ConditionalOperatorClass`)?
No, `BinaryConditionalOperator` is explicitly exempt.
================
Comment at:
clang-tools-extra/clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp:388
+ case Stmt::CXXCatchStmtClass:
+ case Stmt::GotoStmtClass:
+ Reasons |= CognitiveComplexity::Criteria::Increment;
----------------
aaron.ballman wrote:
> Should `IndirectGotoStmtClass` be handled the same way?
Right, it should be.
================
Comment at:
clang-tools-extra/clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp:410
+ case Stmt::CXXCatchStmtClass:
+ case Stmt::LambdaExprClass:
+ Reasons |= CognitiveComplexity::Criteria::IncrementNesting;
----------------
aaron.ballman wrote:
> `BlockExprClass` as well?
>
> Should GNU statement expressions also be treated like a lambda or block?
> BlockExprClass as well?
Thank you for the example, that seems obvious & straight-forward to support.
As discussed, we should handle decls thought, not exprs.
> Should GNU statement expressions also be treated like a lambda or block?
Hm, i guess it should be.
================
Comment at:
clang-tools-extra/clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp:428
+ case Stmt::DoStmtClass:
+ case Stmt::CXXCatchStmtClass:
+ Reasons |= CognitiveComplexity::Criteria::PenalizeNesting;
----------------
aaron.ballman wrote:
> `SEHExceptStmtClass` as well?
I'm very much unfamiliar with that extension.
I would prefer to leave it as-is for now.
================
Comment at:
clang-tools-extra/clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp:473
+ case Decl::CXXConstructor:
+ case Decl::CXXDestructor:
+ break;
----------------
aaron.ballman wrote:
> What about other special member functions like overloaded operators and
> whatnot? How about block declarations?
>
> Namespaces is another one I was curious about but suspect may not want to be
> handled here.
> What about other special member functions like overloaded operators and
> whatnot?
Those are `CXXMethodDecl`, but i'll add a test.
> How about block declarations?
Right, we should handle them, not blockstatements.
> Namespaces is another one I was curious about but suspect may not want to be
> handled here.
To the best of my knowledge, those can only appear at the global scope.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D36836/new/
https://reviews.llvm.org/D36836
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
