From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 28274 invoked by alias); 8 May 2011 02:19:25 -0000 Received: (qmail 28262 invoked by uid 22791); 8 May 2011 02:19:24 -0000 X-SWARE-Spam-Status: No, hits=-6.3 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_HI,SPF_HELO_PASS,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sun, 08 May 2011 02:19:04 +0000 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id p482J4I6013483 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Sat, 7 May 2011 22:19:04 -0400 Received: from [127.0.0.1] (ovpn-113-126.phx2.redhat.com [10.3.113.126]) by int-mx01.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id p482J334025669; Sat, 7 May 2011 22:19:03 -0400 Message-ID: <4DC5FD97.5040705@redhat.com> Date: Sun, 08 May 2011 02:20:00 -0000 From: Jason Merrill User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.15) Gecko/20110421 Fedora/3.1.9-2.fc14 Lightning/1.0b2 Thunderbird/3.1.9 MIME-Version: 1.0 To: Ville Voutilainen CC: gcc-patches@gcc.gnu.org Subject: Re: [PATCH] C++0x, virt-specifier (final/override) support for member functions (parser and analysis both) References: <871v0akr6p.wl%ville@ville-laptop> In-Reply-To: <871v0akr6p.wl%ville@ville-laptop> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit 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 X-SW-Source: 2011-05/txt/msg00591.txt.bz2 On 05/07/2011 07:48 PM, Ville Voutilainen wrote: > +static tree set_virt_specifiers (tree decl, cp_virt_specifiers specifiers) The name of the function needs to be at the beginning of the line. > + else > + virt_specifier = VIRT_SPEC_UNSPECIFIED; > + > + if (!virt_specifier) > + break; Using a named constant one one line and then testing for a boolean value on the next line is unclear. How about just "else break;"? > +/* Non-static member functions have an optional virt-specifier-seq. > + There is a VIRT_SPEC value for each virt-specifier. > + They can be combined by bitwise-or to form the complete set of > + virt-specifiers for a member function. */ > +enum virt_specifier > + { > + VIRT_SPEC_UNSPECIFIED = 0x0, > + VIRT_SPEC_FINAL = 0x1, > + VIRT_SPEC_OVERRIDE = 0x2 > + }; This should be in cp/cp-tree.h. Otherwise, looks pretty good. Jason