Re: [committed][PATCH] Convert sprintf warning code to a dominator walk

[David Malcolm <dmalcolm@redhat.com>]

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?

In particular, AIUI:
"virtual" says: "some subclass might override this method"
"final" says: "no subclass will override this method"

so having both seems contradictory.

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.


Dave