Re: [PATCH] D14824: [PATCH] Add clang-tidy check for static or thread_local objects where construction may throw
aaron.ballman closed this revision.
aaron.ballman added a comment.
Commit in r254415.
Comment at: clang-tidy/utils/Matchers.h:26
@@ -25,1 +25,3 @@
+AST_MATCHER(FunctionDecl, isNoThrow) {
+ const auto *FnTy = Node.getType()->getAs();
alexfh wrote:
> Anything wrong with putting this to the astmatchers library?
I may move that there post-commit because it seems general-purpose enough for
that.
http://reviews.llvm.org/D14824
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D14824: [PATCH] Add clang-tidy check for static or thread_local objects where construction may throw
alexfh accepted this revision.
alexfh added a comment.
This revision is now accepted and ready to land.
LG
Comment at: clang-tidy/utils/Matchers.h:26
@@ -25,1 +25,3 @@
+AST_MATCHER(FunctionDecl, isNoThrow) {
+ const auto *FnTy = Node.getType()->getAs();
Anything wrong with putting this to the astmatchers library?
http://reviews.llvm.org/D14824
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D14824: [PATCH] Add clang-tidy check for static or thread_local objects where construction may throw
alexfh added a comment.
Sorry for the delay, I was sick last week.
Looks mostly fine.
Comment at: clang-tidy/cert/CERTTidyModule.cpp:19
@@ -18,2 +18,3 @@
#include "../misc/ThrowByValueCatchByReferenceCheck.h"
+#include "StaticObjectExceptionCheck.h"
#include "SetLongJmpCheck.h"
Please sort includes.
Comment at: clang-tidy/cert/StaticObjectExceptionCheck.cpp:18
@@ +17,3 @@
+namespace {
+AST_MATCHER(CXXConstructorDecl, isNoThrowConstructor) {
+ const auto *FnTy = Node.getType()->getAs();
ThrownExceptionTypeCheck.cpp defines a similar matcher. Looks like we need to
move the `isNothrow` part to the ASTMatchers.h already.
Comment at: clang-tidy/cert/StaticObjectExceptionCheck.h:18
@@ +17,3 @@
+
+/// FIXME: Write a short description.
+///
Maybe address the FIXME right away? ;)
http://reviews.llvm.org/D14824
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D14824: [PATCH] Add clang-tidy check for static or thread_local objects where construction may throw
aaron.ballman added a comment. Btw, here are some statistics as to the rate at which this diagnostic is triggered: LLVM: 21255 warnings (about 18000 warnings were due to two AST matcher constructors; adding noexcept to those brings the count to 2895 warnings, most of which are cl::opt-related and can be similarly silenced) (2.1MM LoC) Qt: 0 warnings (4.4MM LoC) rethinkdb: 44 warnings (5.7MM LoC) cocos2d: 1339 warnings (984k LoC) opencv: 203 warnings (657k LoC) As best I can tell from random sampling, all of the diagnostics are true positives in that the compiler is unaware of whether the constructor will throw or not. However, many of these true positives come from user-provided constructors where the initializer list does all the work, and the constructor body is empty. In the samples I looked at, the constructors do not throw and can be marked noexcept to silence the warning. However, a few of the constructors can throw and this is a definite true positive instead of a practical false positive. One possible heuristic to silence more of the warnings is to not warn when the constructor called is the default constructor accepting no arguments and the constructor body is trivial, with the belief that default initialization generally does not throw. However, I don't think this diagnostic is chatty enough to warrant that measure when the user can simply add noexcept to the constructor in the event it truly does not throw. http://reviews.llvm.org/D14824 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
