On 11/02/2017 09:33 AM, Markus Trippelsdorf wrote:
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
Yea, I hadn't researched that aspect as thoroughly. ISTM we should
probably pull some of this into our own guidelines.
Jeff
