Bug 23120 – Turn on overloaded virtual functions warning in WebCore

• Status: RESOLVED
• Resolution: WONTFIX
• Component: WebCore Misc.

[Simon Fraser (smfr)]

-Woverloaded-virtual should help us avoid bugs like bug 23082.

[Simon Fraser (smfr)]

diff --git a/WebCore/Configurations/Base.xcconfig b/WebCore/Configurations/Base.xcconfig
index 803911a..9476735 100644
--- a/WebCore/Configurations/Base.xcconfig
+++ b/WebCore/Configurations/Base.xcconfig
@@ -16,6 +16,7 @@ GCC_WARN_ABOUT_DEPRECATED_FUNCTIONS = NO;
 GCC_WARN_ABOUT_MISSING_NEWLINE = YES;
 GCC_WARN_ABOUT_MISSING_PROTOTYPES = YES;
 GCC_WARN_NON_VIRTUAL_DESTRUCTOR = YES;
+GCC_WARN_HIDDEN_VIRTUAL_FUNCTIONS = YES;
 LINKER_DISPLAYS_MANGLED_NAMES = YES;
 PREBINDING = NO;
 VALID_ARCHS = i386 ppc x86_64 ppc64;

[Darin Adler]

The first thing we'd need to do would be to change names in JavaScriptCore for the overloads where functions take either an Identifier or an integer. That probably affects 10 or so functions in JavaScriptCore and would be a simple enough change.

The naming I suggest is to use the word "index" in the names. So for "hasProperty" you could make the version that takes an integer be named "hasIndexProperty".

In fact, I think turning this warning on for JavaScriptCore is a separate step and could almost deserve a separate bug.

[Simon Fraser (smfr)]

The first issue I hit in WebCore was:
    virtual void imageChanged(CachedImage*, const IntRect* = 0);
    virtual void imageChanged(WrappedImagePtr, const IntRect* = 0) { }

[Darin Adler]

(In reply to comment #3)
> The first issue I hit in WebCore was:
>     virtual void imageChanged(CachedImage*, const IntRect* = 0);
>     virtual void imageChanged(WrappedImagePtr, const IntRect* = 0) { }

Seems another case where we can do a name change, but probably one that affects a lot fewer files.

[Cameron Zwarich (cpst)]

I fixed the JSC issues for Clang (which enables this by default) in bug 53760. If anything, I would expect to Clang to be stricter than GCC. Here's a bug to turn the warning on in GCC:

Bug 54369 - Turn on -Woverloaded-virtual for GCC builds of JSC

[Cameron Zwarich (cpst)]

Actually, as I point out in bug 54369, the GCC warning has way too many false positives in Apple's GCC 4.2, and I don't think we could ever turn it on for JSC. The Clang warning is much saner.

[Cameron Zwarich (cpst)]

I'm just going to close this, because the GCC warning is broken and the sane version is on by default in Clang.