alexfh requested changes to this revision.
alexfh added a comment.
This revision now requires changes to proceed.
It looks like you've missed some comments or uploaded a wrong patch.
================
Comment at: clang-tidy/bugprone/ExceptionEscapeCheck.cpp:105
+const TypeVec throwsException(const FunctionDecl *Func) {
+ static thread_local llvm::SmallSet<const FunctionDecl *, 32> CallStack;
+
----------------
alexfh wrote:
> I don't think this is a good use case for a static local variable. If this
> function needs a state, either pass it from the caller or create a utility
> class/struct for it. You can leave a non-recursive entry point with the
> current interface, if you like. For example:
>
> const TypeVec throwsException(const FunctionDecl *Func,
> llvm::SmallSet<const FunctionDecl *, 32> *CallStack) { ... }
>
> const TypeVec throwsException(const FunctionDecl *Func) {
> llvm::SmallSet<const FunctionDecl *, 32> CallStack;
> return throwsException(Func, &CallStack);
> }
>
Still not addressed?
================
Comment at: clang-tidy/bugprone/ExceptionEscapeCheck.cpp:110
+
+ if (const auto *Body = Func->getBody()) {
+ CallStack.insert(Func);
----------------
alexfh wrote:
> Please use the concrete type here, since it's not obvious from the context.
Still not addressed?
================
Comment at: clang-tidy/bugprone/ExceptionEscapeCheck.cpp:119
+ if (const auto *FPT = Func->getType()->getAs<FunctionProtoType>()) {
+ for (const auto Ex : FPT->exceptions()) {
+ Result.push_back(&*Ex);
----------------
alexfh wrote:
> Please use the concrete type here.
Still not addressed?
================
Comment at: clang-tidy/bugprone/ExceptionEscapeCheck.cpp:120
+ for (const auto Ex : FPT->exceptions()) {
+ Result.push_back(&*Ex);
+ }
----------------
alexfh wrote:
> Ex.getTypePtrOrNull() / Ex.getTypePtr() would be easier to understand here.
Still not addressed?
https://reviews.llvm.org/D33537
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
