Re: [committed][PATCH] Trivial cleanups to new classes

[Markus Trippelsdorf <markus at trippelsdorf dot de>]

On 2017.11.02 at 08:55 -0600, Jeff Law wrote:
> 
> As has been discussed on-list.  This patch adds a virtual destructor to 
> the new classes in tree-ssa-propagate.h per our coding conventions and 
> what are considered best practices.  It doesn't matter for any code I'm 
> aware of today -- it's a defensive measure.
> 
> This also drops the "virtual" keyword on the FINAL OVERRIDE member 
> functions in gimple-ssa-sprintf's sprintf_dom_walker class.  Opinions 
> here are more mixed.  It's agreed that the keyword is redundant in this 
> context.  The question is whether or not it adds confusion or reduces 
> confusion.
> 
> The virtual keyword intuitively implies to me the member can be 
> overridden by a derived class, but that's in direct conflict with the 
> FINAL keyword.
> 
> Others focus more on the fact that the virtual keyword implies that the 
> calls are typically indirect.   But in the case of a FINAL, one of the 
> hopes is that devirt can use the information to change the indirect call 
> into a direct call.
> 
> In the end the arguments for dropping the "virtual" seemed stronger to me.
> 
> Bootstrapped and regression tested on x86.  Installing on the trunk.

Even specifying both override and final is normally frowned upon, see:
https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#c128-virtual-functions-should-specify-exactly-one-of-virtual-override-or-final

-- 
Markus