Bug 54221 - Fix clang warning "WebCore::HTMLAreaElement::getRect' hides overloaded virtual function [-Woverloaded-virtual]"
Summary: Fix clang warning "WebCore::HTMLAreaElement::getRect' hides overloaded virtua...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Nico Weber
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-02-10 10:32 PST by Nico Weber
Modified: 2011-02-10 20:23 PST (History)
5 users (show)

See Also:


Attachments
Patch (5.50 KB, patch)
2011-02-10 10:35 PST, Nico Weber
no flags Details | Formatted Diff | Diff
Patch (5.12 KB, patch)
2011-02-10 11:54 PST, Nico Weber
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Nico Weber 2011-02-10 10:32:59 PST
Fix clang warning "WebCore::HTMLAreaElement::getRect' hides overloaded virtual function [-Woverloaded-virtual]"
Comment 1 Nico Weber 2011-02-10 10:35:09 PST
Created attachment 82000 [details]
Patch
Comment 2 Nico Weber 2011-02-10 10:36:54 PST
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;
                    ^
Comment 3 James Robinson 2011-02-10 11:31:33 PST
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.
Comment 4 Nico Weber 2011-02-10 11:37:13 PST
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.
Comment 5 Darin Adler 2011-02-10 11:38:37 PST
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.
Comment 6 Nico Weber 2011-02-10 11:54:23 PST
Created attachment 82019 [details]
Patch
Comment 7 WebKit Commit Bot 2011-02-10 13:04:57 PST
Comment on attachment 82019 [details]
Patch

Clearing flags on attachment: 82019

Committed r78261: <http://trac.webkit.org/changeset/78261>
Comment 8 WebKit Commit Bot 2011-02-10 13:05:01 PST
All reviewed patches have been landed.  Closing bug.