[PATCH] D18575: [clang-tidy] New checker to replace deprecated throw() specifications

[don hinton via cfe-commits]

hintonda added inline comments.

================
Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:21
@@ +20,3 @@
+
+static StringRef
+makeDynamicExceptionString(const SourceManager &SM,
----------------
aaron.ballman wrote:
> Instead of a bunch of static functions, would an unnamed namespace make more sense?
Just following the pattern established in other checkers so as to minimize the number of changes, but will change to namespace.

================
Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:34
@@ +33,3 @@
+
+static CharSourceRange makeMoveRange(const SourceManager &SM,
+                                     const LangOptions &LangOps,
----------------
aaron.ballman wrote:
> Since this function is only called one time and is a one-liner, perhaps it makes more sense to inline the body at the call site?
Was a bit more involved, but last round of changes simplified it.  Will simplify even more.

================
Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:46
@@ +45,3 @@
+                       const SmallVector<Token, 16> &Tokens) {
+  // Find throw token -- it's a keyword, so there can't be more than one.  Also,
+  // it should be near the end of the declaration, so search from the end.
----------------
aaron.ballman wrote:
> Pathologically terrible counter-case: `void func() throw(decltype(throw 12) *)`
Good point, looks like I need a full fledged parser to catch 100% or the cases -- or we could ignore these corner cases.

================
Comment at: clang-tidy/modernize/UseNoexceptCheck.h:22
@@ +21,3 @@
+/// \brief Replace dynamic exception specifications, with
+/// `noexcept` (or user-defined macro) or `noexcept(true)`.
+/// \code
----------------
aaron.ballman wrote:
> I think this comment means `noexcept(false)` instead? Or is there a reason to replace with `noexcept(true)` instead of just `noexcept`?
Typo, should just be noexcept.


http://reviews.llvm.org/D18575