aaron.ballman added inline comments.

================
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):
----------------
alexfh wrote:
> aaron.ballman wrote:
> > alexfh wrote:
> > > 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.
> > I am more curious as to your comfort level about committing a fixit that we 
> > know will break working code. ;-)
> > 
> > As for the suggested approach, I would obviously cover that with Richard, 
> > but I think we're going to need exception specifications more now that 
> > they're going to be part of the type system for C++. We have 
> > `FunctionProtoTypeLoc` for tracking this information.
> The important question here is how frequently is this likely to happen. I 
> guess, it depends on the codebase, but maybe you have a rough guess?
Multiple uses of `throw` within a dynamic exception specification? Rare. Paren 
use that doesn't match what we expect? Infrequent, but likely to generate a bug 
report at some point depending on how often people use clang-tidy to check 
extensive code bases that actually *use* dynamic exception specifications. So 
my biggest concern really boils down to the fact that we're assuming the first 
right paren we come to after a "throw" keyword in a dynamic exception 
specification is the terminating right paren. I think we need to use paren 
matching instead. It would be nice if we could use `BalancedDelimiterTracker` 
to solve this, come to think of it.


http://reviews.llvm.org/D18575



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to