[PATCH] Supporting function-try-blocks for -Wreturn-type

Richard Smith richard at metafoo.co.uk
Wed Oct 22 17:13:24 PDT 2014


On Wed, Oct 22, 2014 at 2:44 PM, Aaron Ballman <aaron at aaronballman.com>
wrote:

> On Wed, Oct 22, 2014 at 5:21 PM, David Blaikie <dblaikie at gmail.com> wrote:
> > My only quandry is: what other things can the body be? If CompoundStmt
> and
> > and CXXTryStmt are the only ones, then we can remove the conditional
> > entirely.
>
> I think we can remove that conditional. IssueWarnings calls getBody()
> on the Decl passed in from PopFunctionScopeInfo, and that should
> always be something function-like, and I can't think of a situation
> where we'd get something other than a CompoundStmt or CXXTryStmt.
>
> Richard, can you think of any situations that would be problematic?


No, the body of a function is always one of those two things.

~Aaron
> >
> > On Wed, Oct 22, 2014 at 2:18 PM, Aaron Ballman <aaron at aaronballman.com>
> > wrote:
> >>
> >> On Wed, Oct 22, 2014 at 5:08 PM, David Blaikie <dblaikie at gmail.com>
> wrote:
> >> >
> >> >
> >> > On Wed, Oct 22, 2014 at 2:01 PM, Aaron Ballman <
> aaron at aaronballman.com>
> >> > wrote:
> >> >>
> >> >> On Wed, Oct 22, 2014 at 4:56 PM, David Blaikie <dblaikie at gmail.com>
> >> >> wrote:
> >> >> > Is a CompoundStatement's start/end loc ever different from
> >> >> > lbrace/rbrace
> >> >> > location? (maybe when it's a compound statement without braces? But
> >> >> > that
> >> >> > can
> >> >> > never occur in this part of the AST, right)
> >> >>
> >> >> It appears to be possible because you can call setLBracLoc (thank
> >> >> goodness we left the "e" off there...), but that looks to only be
> >> >> called from ASTReader,
> >> >>
> >> >> > If it isn't, then the if/else
> >> >> > LBrace/RBrace bit could be omitted.
> >> >>
> >> >> There's no relationship between compound statements and function try
> >> >> block statements, so I'm not certain how we would omit the if/else.
> >> >
> >> >
> >> > Not sure I follow - but CompoundStmt and CXXTryStmt are both Stmts and
> >> > all
> >> > Stmts have getLocStart/End, right? At least that's my theory...
> >>
> >> Oh, hey, would you look at that? It does, and I was simply not seeing
> >> it when I looked. Yes, that's much more clean -- attached patch uses
> >> that approach.
> >>
> >> Thanks!
> >>
> >> ~Aaron
> >
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20141022/cf381108/attachment.html>


More information about the cfe-commits mailing list