Bug 54221 – Fix clang warning "WebCore::HTMLAreaElement::getRect' hides overloaded virtual function [-Woverloaded-virtual]"

• Component: New Bugs
• Status: RESOLVED FIXED

[Nico Weber]

Fix clang warning "WebCore::HTMLAreaElement::getRect' hides overloaded virtual function [-Woverloaded-virtual]"

[Nico Weber]

Created attachment 82000 [details]
Patch

[Nico Weber]

Full warning:

third_party/WebKit/Source/WebCore/html/HTMLAreaElement.h:44:13:error: 'WebCore::HTMLAreaElement::getRect' hides overloaded virtual function [-Woverloaded-virtual]
    IntRect getRect(RenderObject*) const;
            ^
third_party/WebKit/Source/WebCore/dom/ContainerNode.h:63:21: note: hidden overloaded virtual function 'WebCore::ContainerNode::getRect' declared here
    virtual IntRect getRect() const;
                    ^

[James Robinson]

I'm not completely sure this is avoiding a (potential) bug - looks like getRect() isn't intended to be an override and callsites are never ambiguous.  It is a little confusing, though.

[Nico Weber]

Yes, it's not a real bug in this case, just confusing. But fixing this makes it possible to turn on -Woverride-virtual in clang (and this warning is on by default with -Wall), which does find real bugs.

Even since it's not a bug in this case, it's a bit confusing to have a non-override with the same name as you say, so changing this seems beneficial in general.

[Darin Adler]

Comment on attachment 82000 [details]
Patch

I think a rename is totally fine.

But I don’t think that adding a “for renderer” suffix is the best rename. For one thing, having “get” in these names doesn’t match our usual naming scheme. So the Node::getRect function also has a bad name for a WebKit function.

I think I’d name them computeRect and computePath.

Since the patch isn’t applying with the EWS I think we need a new version. review- because of the EWS issue.

[Nico Weber]

Created attachment 82019 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 82019 [details]
Patch

Clearing flags on attachment: 82019

Committed r78261: <http://trac.webkit.org/changeset/78261>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.