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

[Aaron Ballman via cfe-commits]

aaron.ballman added inline comments.

================
Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:21
@@ +20,3 @@
+
+static StringRef
+makeDynamicExceptionString(const SourceManager &SM,
----------------
hintonda wrote:
> 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.
I don't have strong opinions; I think the usual rule of thumb boils down to whether the code fits on a page or not (if it does, use an unnamed namespace, if not, use static functions).

================
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.
----------------
hintonda wrote:
> 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.
At the very least, we should have test cases showing what the behavior is with a big FIXME around this code, should you decide to keep it. I'm not keen on the idea of this being part of a fixit that may destroy well-defined user code. Same for the assumptions about the location of right parens. That code looks equally broken even without multiple `throw` tokens in the stream. Consider:
`void func() throw(int(int));`


http://reviews.llvm.org/D18575