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
