[PATCH] D20693: [clang-tidy] New checker to replace dynamic exception specifications

Alexander Kornienko via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Jan 10 15:49:43 PST 2017

alexfh requested changes to this revision.
alexfh added inline comments.
This revision now requires changes to proceed.

Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:99
+  assert(CRange.isValid() && "Exception Specification Range is invalid.");
+  assert(FnTy && "FunctionProtoType is null.");
I suspect there might be valid code that will trigger this assertion. I would issue the diagnostic, just without a FixItHint.

Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:111
+  diag(Range.getBegin(), "found dynamic exception specification '%0'")
+      << makeDynamicExceptionString(*Result.SourceManager, CRange) << FixIt;
The message neither explains what's wrong with the code, nor says what is a better alternative. Something like "dynamic exception specification '%0' is so 1998 <actually explain what's wrong with it>; consider using 'noexcept(...)' <real suggestion>" would lead to less user confusion.

Comment at: docs/clang-tidy/checks/modernize-use-noexcept.rst:6-8
+The check converts dynamic exception specifications, e.g.,
+``throw()``, ``throw(<exception>[,...])``, or ``throw(...)``, to
+``noexcept``, ``noexcept(false)``, blank, or a user defined macro.
alexfh wrote:
> This description doesn't say why `noexcept` is better.
This still needs to be addressed.


More information about the cfe-commits mailing list