From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 128466 invoked by alias); 5 Dec 2018 23:02:39 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 127715 invoked by uid 89); 5 Dec 2018 23:02:38 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.2 spammy=Nice X-HELO: gate.crashing.org Received: from gate.crashing.org (HELO gate.crashing.org) (63.228.1.57) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 05 Dec 2018 23:02:35 +0000 Received: from gate.crashing.org (localhost.localdomain [127.0.0.1]) by gate.crashing.org (8.14.1/8.14.1) with ESMTP id wB5N2HVS022583; Wed, 5 Dec 2018 17:02:17 -0600 Received: (from segher@localhost) by gate.crashing.org (8.14.1/8.14.1/Submit) id wB5N2Ecc022582; Wed, 5 Dec 2018 17:02:14 -0600 Date: Wed, 05 Dec 2018 23:02:00 -0000 From: Segher Boessenkool To: Jason Merrill Cc: gcc-patches@gcc.gnu.org, jakub@redhat.com, Joseph Myers , nathan@acm.org, polacek@redhat.com Subject: Re: [PATCH 1/2] asm qualifiers (PR55681) Message-ID: <20181205230213.GV3803@gate.crashing.org> References: <74db3dde365a2a30fc5d4779ef67278a2b29ca1a.1543766396.git.segher@kernel.crashing.org> <0f5fe39e-4061-1b0b-bb5c-3676f395bff6@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <0f5fe39e-4061-1b0b-bb5c-3676f395bff6@redhat.com> User-Agent: Mutt/1.4.2.3i X-IsSubscribed: yes X-SW-Source: 2018-12/txt/msg00321.txt.bz2 Hi! On Wed, Dec 05, 2018 at 04:47:37PM -0500, Jason Merrill wrote: > On 12/2/18 11:38 AM, Segher Boessenkool wrote: > >PR55681 observes that currently only one qualifier is allowed for > >inline asm, so that e.g. "volatile asm" is allowed, "const asm" is also > >okay (with a warning), but "const volatile asm" gives an error. Also > >"goto" has to be last. > > > >This patch changes things so that only "asm-qualifiers" are allowed, > >that is "volatile" and "goto", in any combination, in any order, but > >without repetitions. > > > > > >2018-12-02 Segher Boessenkool > > > > PR inline-asm/55681 > > * doc/extend.texi (Basic Asm): Update grammar. > > (Extended Asm): Update grammar. > > > >gcc/c/ > > PR inline-asm/55681 > > * c-parser.c (c_parser_for_statement): Update grammar. Allow any > > combination of volatile and goto, in any order, without repetitions. > > > >gcc/cp/ > > PR inline-asm/55681 > > * parser.c (cp_parser_using_directive): Update grammar. Allow any > > combination of volatile and goto, in any order, without repetitions. > > You don't actually change cp_parser_using_directive, despite what diff > says: you're changing cp_parser_asm_definition. I trust diff too much, sigh. > >+ for (bool done = false; !done ; ) > >+ switch (cp_lexer_peek_token (parser->lexer)->keyword) > >+ { > >+ case RID_VOLATILE: > >+ if (!volatile_p) > >+ { > >+ /* Remember that we saw the `volatile' keyword. */ > >+ volatile_p = true; > >+ /* Consume the token. */ > >+ cp_lexer_consume_token (parser->lexer); > >+ } > >+ else > >+ done = true; > >+ break; > >+ case RID_GOTO: > >+ if (!goto_p && parser->in_function_body) > >+ { > >+ /* Remember that we saw the `goto' keyword. */ > >+ goto_p = true; > >+ /* Consume the token. */ > >+ cp_lexer_consume_token (parser->lexer); > >+ } > >+ else > >+ done = true; > >+ break; > >+ default: > >+ done = true; > >+ } > > An arguably simpler alternative to using the 'done' variable would be to > 'break' out of the loop after the switch, and have the consume_token > cases explicitly 'continue'. Yeah, that is neater, continue only deals with the loop. Nice. > We also might remember the old tokens and give a more helpful error > message in the case of duplicate keywords. > > But I won't insist on either of these, the C++ changes are OK as-is. I'll commit it like this then, and work on the improvements afterwards (they also apply to the C frontend). Thanks for the review! Segher