[cfe-commits] [Patch] Add warning for semi-colon after class function definition

[Richard Trieu]

On Mon, Feb 13, 2012 at 3:13 PM, David Blaikie <dblaikie at gmail.com> wrote:

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

Yup, that's also possible.  It's usually a simple check like:

if (Tok.is(tok::semi)) {
  // Emit Diagnostic
  ConsumeToken();
}

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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120213/1d189488/attachment.html>