carlosgalvezp added a comment.
Looks good, great to see all these issues fixed! Have a couple small comments.
================
Comment at: clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp:319
+static bool cannotThrow(const FunctionDecl *Func) {
+ const auto *FunProto = Func->getType()->getAs<FunctionProtoType>();
----------------
PiotrZSL wrote:
> isuckatcs wrote:
> > Put this in the anonymous namespace above please to remain consistent.
> well, llvm style require `static`, if we want to be consistent, I can change
> all other into static later.
Nit: I personally find it more readable as "canThrow". People with issues
reading double negations will probably appreciate that too :) But if you
strongly prefer this name I won't oppose.
================
Comment at: clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp:330-332
+ return NoexceptExpr && !NoexceptExpr->isValueDependent() &&
+ NoexceptExpr->EvaluateAsBooleanCondition(
+ Result, Func->getASTContext(), true) &&
----------------
Would be good to write a small comment documenting this logic.
================
Comment at: clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp:332
+ NoexceptExpr->EvaluateAsBooleanCondition(
+ Result, Func->getASTContext(), true) &&
+ Result;
----------------
Would be good to use the syntax `/* Parameter = */true` to document what this
magic `true` means.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D153458/new/
https://reviews.llvm.org/D153458
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
