From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 56009 invoked by alias); 30 Oct 2017 01:41:56 -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 55910 invoked by uid 89); 30 Oct 2017 01:41:55 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.2 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.2 spammy=FINAL, Hx-languages-length:2059, H*M:ball, super X-HELO: paperclip.tbsaunde.org Received: from tbsaunde.org (HELO paperclip.tbsaunde.org) (66.228.47.254) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 30 Oct 2017 01:41:54 +0000 Received: from ball (pool-98-116-146-128.nycmny.fios.verizon.net [98.116.146.128]) by paperclip.tbsaunde.org (Postfix) with ESMTPSA id 02D39C082; Mon, 30 Oct 2017 01:41:50 +0000 (UTC) Date: Mon, 30 Oct 2017 02:03:00 -0000 From: Trevor Saunders To: David Malcolm Cc: Jeff Law , gcc-patches Subject: Re: [committed][PATCH] Convert sprintf warning code to a dominator walk Message-ID: <20171030014110.bwvmymber564n7n7@ball> References: <1509127439.15469.16.camel@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1509127439.15469.16.camel@redhat.com> User-Agent: NeoMutt/20170609 (1.8.3) X-SW-Source: 2017-10/txt/msg02156.txt.bz2 On Fri, Oct 27, 2017 at 02:03:59PM -0400, David Malcolm wrote: > On Fri, 2017-10-27 at 10:55 -0600, Jeff Law wrote: > > Prereq for eventually embedding range analysis into the sprintf > > warning > > pass. The only thing that changed since the original from a few days > > ago was the addition of FINAL OVERRIDE to the before_dom_children > > override function. > > > > Re-bootstrapped and regression tested on x86. > > > > Installing on the trunk. Final patch attached for archival purposes. > > > > > > Jeff > > Sorry to be re-treading the FINAL/OVERRIDE stuff, but... > > [...snip...] > > > +class sprintf_dom_walker : public dom_walker > > +{ > > + public: > > + sprintf_dom_walker () : dom_walker (CDI_DOMINATORS) {} > > + ~sprintf_dom_walker () {} > > + > > + virtual edge before_dom_children (basic_block) FINAL OVERRIDE; > > Is it just me, or is it a code smell to have both "virtual" and > "final"/"override" on a decl? fwiw I'm of the opinion that all decls of a virtual function should be marked as virtual including ones also marked as final or override. > In particular, AIUI: > "virtual" says: "some subclass might override this method" I'd say virtual means that the function is generally called through a vtable not directly, and that it may be overriding a super class method and / or allow subclasses to override this function. > "final" says: "no subclass will override this method" > > so having both seems contradictory. I guess that depends on how you interpret virtual. > If sprintf_dom_walker is providing a implementation of a vfunc of > dom_walker, then presumably this should just lose the "virtual" on the > subclass, it's presumably already got the "virtual" it needs in the > base class. Its certainly not necessary, but I think its good to be explicit. Especially if you mark the class as final instead of individual methods it isn't obvious the method gets called indirectly without checking the parent classes for declarations. Trev > > > Dave