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