On Mon, Feb 13, 2012 at 3:58 PM, Richard Trieu <[email protected]> wrote: > On Mon, Feb 13, 2012 at 3:13 PM, David Blaikie <[email protected]> wrote: >> >> On Mon, Feb 13, 2012 at 2:37 PM, Richard Trieu <[email protected]> wrote: >> > On Mon, Feb 13, 2012 at 1:59 PM, David Blaikie <[email protected]> >> > wrote: >> >> >> >> On Mon, Feb 13, 2012 at 1:52 PM, Richard Trieu <[email protected]> >> >> wrote: >> >> > On Mon, Jan 30, 2012 at 10:40 AM, Richard Trieu <[email protected]> >> >> > wrote: >> >> >> >> >> >> On Wed, Jan 25, 2012 at 5:24 PM, Richard Trieu <[email protected]> >> >> >> 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.
Fair enough - though I'm not sure if a generic error message would be terribly problematic (do you really need to know that this particular spurious semi is after a member function definition rather than at the global scope?) If the checks are that regular it'd probably make sense to add a utility function to sema (similar to the brace handling, etc) to conusme - with an optional diagnostic to emit & a default to use otherwise. But yeah, I suppose the essential answer to my question, as it pertains to this CR is "no, there's not one place we can fix this at the moment - but a pretty common cliche we can sweep to cleanup if we want to spend the tiime at some ponit" Thanks, - David _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
