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. https://reviews.llvm.org/D20693 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
