On Tue, Feb 14, 2012 at 11:10 AM, Richard Trieu <[email protected]> wrote:
> On Mon, Feb 13, 2012 at 4:05 PM, David Blaikie <[email protected]> wrote: > >> 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 >> > > Took some of David's suggestions and modified the patch. Condensed all > the extra semi warnings into a single diagnostic message with a %select and > with the flag -Wextra-semi. C++98 compat warning was kept separate. Moved > the emitting of logic and semi consumption to a shared function. I also > added logic so that lines of semi-colons now generate a single warning > instead of a warning per semi-colon. > Ping.
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
