[cfe-commits] [Patch] New warning for scopes that start in one file and end in another.

[Richard Trieu]

On Mon, Mar 26, 2012 at 5:05 PM, David Blaikie <dblaikie at gmail.com> wrote:

> On Mon, Mar 26, 2012 at 2:49 PM, Richard Trieu <rtrieu at google.com> wrote:
> > On Tue, Feb 21, 2012 at 8:47 PM, Richard Trieu <rtrieu at google.com>
> wrote:
> >>
> >> On Fri, Feb 17, 2012 at 4:51 AM, Manuel Klimek <klimek at google.com>
> wrote:
> >>>
> >>> On Thu, Feb 16, 2012 at 5:31 PM, Matt Beaumont-Gay <
> matthewbg at google.com>
> >>> wrote:
> >>> > On Thu, Feb 16, 2012 at 02:46, Sebastian Redl
> >>> > <sebastian.redl at getdesigned.at> wrote:
> >>> >>
> >>> >> On 16.02.2012, at 08:11, Matt Beaumont-Gay wrote:
> >>> >>
> >>> >>> On Wed, Feb 15, 2012 at 20:47, Eli Friedman <
> eli.friedman at gmail.com>
> >>> >>> wrote:
> >>> >>>> Mechanically, the patch looks fine...
> >>> >>>>
> >>> >>>> What sort of testing have you done with this patch?  Has this
> caught
> >>> >>>> any interesting issues in existing code?  If not, why is this
> >>> >>>> warning
> >>> >>>> useful?  Any false positives we should be aware of?
> >>> >>>
> >>> >>> I imagine that this will not have a lot of hits on checked-in code.
> >>> >>> However, I can see it being very useful during development, e.g.
> when
> >>> >>> you forget to close a namespace at the bottom of a header file.
> >>> >>
> >>> >> But you'd most likely get an error anyway, complaining about
> unmatched
> >>> >> braces.
> >>> >
> >>> > Maybe, maybe not -- failing to close a namespace can cause a lot of
> >>> > cascading errors, which could easily hit -ferror-limit.
> >>>
> >>> How does this patch address that? It's still only warning on the
> >>> closing brace, right?
> >>>
> >>> Cheers,
> >>> /Manuel
> >>>
> >>  This patch moves the checking from the parser to the lexer.  This
> allows
> >> the error to be emitted at the end of the file instead of at the closing
> >> brace.  Since it doesn't know where the closing brace is, the warning
> will
> >> only point to the opening brace.
> >
> >
> > Ping.  And this time, to the list and not just to Manuel.
>
> Seems reasonable to me (assuming someone else has already asked about
> (& you've verified) the perf impact). My only (optional) feedback is:
>
> * The test case seems a bit subtle (using an expected-warning in a cpp
> file that actually appears in the header - it seems almost like a bug
> in -verify that that works at all) & perhaps a FileCheck verification
> would be better.
>
> * The SmallVector of nestings might do well with a higher initial size
> - since there's only one anyway. But it's not likely to thrash too
> much at least, just grow to the max depth & then sit there.
>
> - David
>

Another setback.  When the PP does a backtrack, it throws the brace count
off.  Need to find a more robust solution for this warning.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120418/09f27ec4/attachment.html>