[PATCH] D153458: [clang-tidy] Model noexcept more properly in bugprone-exception-escape

[Carlos Galvez via Phabricator via cfe-commits]

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