<div class="gmail_quote">On Mon, Feb 13, 2012 at 3:13 PM, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">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><div>On Mon, Feb 13, 2012 at 2:37 PM, Richard Trieu <<a href="mailto:rtrieu@google.com" target="_blank">rtrieu@google.com</a>> wrote:<br>
> On Mon, Feb 13, 2012 at 1:59 PM, David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>> wrote:<br>
>><br>
>> On Mon, Feb 13, 2012 at 1:52 PM, Richard Trieu <<a href="mailto:rtrieu@google.com" target="_blank">rtrieu@google.com</a>> wrote:<br>
>> > On Mon, Jan 30, 2012 at 10:40 AM, Richard Trieu <<a href="mailto:rtrieu@google.com" target="_blank">rtrieu@google.com</a>><br>
>> > wrote:<br>
>> >><br>
>> >> On Wed, Jan 25, 2012 at 5:24 PM, Richard Trieu <<a href="mailto:rtrieu@google.com" target="_blank">rtrieu@google.com</a>><br>
>> >> wrote:<br>
>> >>><br>
>> >>> This patch adds a warning to catch semi-colons after function<br>
>> >>> definitions.  The motivation of removing extraneous semi-colons is to<br>
>> >>> make<br>
>> >>> it easier to see which scopes are being ended by a brace.  Function<br>
>> >>> scopes<br>
>> >>> will end with a plain brace.  Class scopes will end with brace and<br>
>> >>> semi-colon.<br>
>> >>><br>
>> >>> class C {<br>
>> >>>   void F() {};<br>
>> >>> };<br>
>> >>><br>
>> >>> extra-semi.cc:2:14: warning: extra ';' after function definition<br>
>> >>> [-Wextra-semi]<br>
>> >>>   void F() {};<br>
>> >>>              ^<br>
>> >>> 1 warning generated.<br>
>> >><br>
>> >><br>
>> >> Ping.<br>
>> ><br>
>> > Ping.<br>
>><br>
>> Not to derail this review - but just out of curiosity: do we have a<br>
>> more general 'unnecessary ;' warning? or could we make a general one<br>
>> that covers all cases where ';' is just redudndant & suggests a<br>
>> removal?<br>
><br>
> There's a few in the parse diagnostics.  They are:<br>
><br>
> ext_top_level_semi<br>
> warn_cxx98_compat_top_level_semi<br>
> ext_extra_struct_semi<br>
> ext_extra_ivar_semi<br>
><br>
> warn_cxx98_compat_top_level_semi is a warning that is in a diagnostic group<br>
> so it must be left separate.  The other 3 are Extensions and could probably<br>
> be folded in with my diagnostic if I switched mine to Extension as well and<br>
> added a select into the message.<br>
<br>
<br>
</div></div>Oh, sorry - I wasn't so much concerned with the diagnostic text, as<br>
the logic - is there some way we could, in a relatively<br>
universal/general way, catch all/most unnecessary semicolons rather<br>
than adding it on a case-by-case basis like this?<br>
</blockquote></div><br><div>Yup, that's also possible.  It's usually a simple check like:</div><div><br></div><div>if (Tok.is(tok::semi)) {</div><div>  // Emit Diagnostic</div><div>  ConsumeToken();</div><div>}</div>

<div><br></div><div>And replace it with a ConsumeExtraSemi function call with a partial diagnostic argument.  Then we can add some more robust logic into semi checking.  But we'll still need to find every check for a semi-colon token and add this check there, mainly so the diagnostic has enough context to emit the right error message.</div>