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

[Alexander Kornienko via cfe-commits]

alexfh added inline comments.

================
Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:22
@@ +21,3 @@
+namespace {
+
+StringRef makeDynamicExceptionString(const SourceManager &SM,
----------------
It's much more common for LLVM code to use static functions:

http://llvm.org/docs/CodingStandards.html#anonymous-namespaces
"make anonymous namespaces as small as possible, and only use them for class declarations" 

================
Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:39
@@ +38,3 @@
+
+  // FIXME: Add paren matching so we can parse more complex throw statements,
+  // e.g., (examples provided by Aaron Ballman):
----------------
hintonda wrote:
> aaron.ballman wrote:
> > @alexfh, what are your feelings on this FIXME? I am a bit concerned because these examples will cause the replacement range to be incorrect, which will turn working code into ill-formed code. The alternative, as I see it, is to instead properly track the exception specification source range information as part of the FunctionDecl (akin to `FunctionDecl::getReturnTypeSourceRange()`).
> Btw, I'm working on a fix I believe will handle all cases -- plan to checkin later today.  However, it won't be that efficient unless I can find a way to match params that contain dynamic exception specifications.  If they are only legal for function pointers -- which I think is the case -- that would make it easy and efficient, i.e., I wouldn't have to match all FunctionDecl's with one or more parameter and test them.
> 
> Is it possible to match a parameter that is a function pointer?
> The alternative, as I see it, is to instead properly track the exception specification source range information as part of the FunctionDecl (akin to FunctionDecl::getReturnTypeSourceRange()).

It's all trade-offs: adding this information to the AST allows certain tasks in tooling to be done easier. On the other hand, this leads to bloating of the AST, which can already be huge. In this specific case, always keeping the source range of the exception specifier might be wasteful, since exception specifiers are rather rare (in my world, at least). But there might be a way to optionally store this information, e.g. in the `ASTContext`. In any case, makes sense to ask Richard Smith.


http://reviews.llvm.org/D18575