alexfh requested changes to this revision.
alexfh added a comment.
This revision now requires changes to proceed.
Thank you for the updates. A few more comments.
================
Comment at: clang-tidy/bugprone/ExceptionEscapeCheck.cpp:150
+ } else if (const auto *Try = dyn_cast<CXXTryStmt>(St)) {
+ auto Uncaught = throwsException(Try->getTryBlock(), Caught);
+ for (unsigned i = 0; i < Try->getNumHandlers(); ++i) {
----------------
alexfh wrote:
> Please use the concrete type here and below.
> http://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable
> says
>
> > Don’t “almost always” use auto, but do use auto with initializers like
> > cast<Foo>(...) or other places where the type is already obvious from the
> > context.
I don't remember where this comment was, but now I see a few more instances of
the same issue. Specifically, ThrownExpr and ThrownType here and CaughtType
below.
================
Comment at: clang-tidy/bugprone/ExceptionEscapeCheck.cpp:17
+
+using namespace clang;
+using namespace clang::ast_matchers;
----------------
You can open namespace clang here instead of this using directive.
================
Comment at: clang-tidy/bugprone/ExceptionEscapeCheck.cpp:22
+typedef llvm::SmallVector<const Type *, 8> TypeVec;
+typedef llvm::SmallSet<StringRef, 8> StringSet;
+} // namespace
----------------
There's `llvm::StringSet<>` in llvm/ADT/StringSet.h.
================
Comment at: clang-tidy/bugprone/ExceptionEscapeCheck.cpp:25
+
+static const TypeVec throwsException(const FunctionDecl *Func);
+
----------------
Too many forward declarations for my taste. Can you just move all static
functions here and remove unnecessary forward declarations? I guess, one should
be enough for all the functions here.
================
Comment at: clang-tidy/bugprone/ExceptionEscapeCheck.cpp:178
+ }
+ auto *NewEnd =
+ llvm::remove_if(Uncaught, [&CaughtType](const Type *ThrownType) {
----------------
iterator being a pointer is an implementation detail of SmallVector. s/auto
\*/auto /
================
Comment at: docs/ReleaseNotes.rst:230
+<<<<<<< HEAD
+=======
----------------
Resolve the conflicts.
================
Comment at: docs/clang-tidy/checks/bugprone-exception-escape.rst:28
+
+ Comma separated list containing function names which should not throw. An
+ example for using this parameter is the function ``WinMain()`` in the
----------------
baloghadamsoftware wrote:
> dberris wrote:
> > `EnabledFunctions` but they *should not* throw?
> Maybe we should come up with a better name for this option. I just took this
> from our company internal check.
FunctionsThatShouldNotThrow?
================
Comment at: docs/clang-tidy/checks/bugprone-exception-escape.rst:29
+ Comma separated list containing function names which should not throw. An
+ example for using this parameter is the function ``WinMain()`` in the
+ Windows API. Default vale is empty string.
----------------
Examples should be copy-pastable. I suppose, one can't use `WinMain()` verbatim
as the value of this option. An example could be: `main,WinMain` or something
like that.
================
Comment at: docs/clang-tidy/checks/bugprone-exception-escape.rst:30
+ example for using this parameter is the function ``WinMain()`` in the
+ Windows API. Default vale is empty string.
+
----------------
s/vale/value/
https://reviews.llvm.org/D33537
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
