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