23120 2009-01-05 11:44:06 -0800 Turn on overloaded virtual functions warning in WebCore 2011-04-17 20:57:48 -0700 1 1 1 Unclassified WebKit WebCore Misc. 528+ (Nightly build) Mac OS X 10.5 RESOLVED WONTFIX P2 Normal --- 23352 1 simon.fraser webkit-unassigned andersca darin sam zwarich oldest_to_newest 104459 0 simon.fraser 2009-01-05 11:44:06 -0800 -Woverloaded-virtual should help us avoid bugs like bug 23082. 104464 1 simon.fraser 2009-01-05 11:53:33 -0800 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; 105932 2 darin 2009-01-15 11:37:29 -0800 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. 105934 3 simon.fraser 2009-01-15 11:41:19 -0800 The first issue I hit in WebCore was: virtual void imageChanged(CachedImage*, const IntRect* = 0); virtual void imageChanged(WrappedImagePtr, const IntRect* = 0) { } 105935 4 darin 2009-01-15 11:43:18 -0800 (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. 350476 5 zwarich 2011-02-13 16:08:31 -0800 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 350480 6 zwarich 2011-02-13 16:22:52 -0800 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. 387510 7 zwarich 2011-04-17 20:57:48 -0700 I'm just going to close this, because the GCC warning is broken and the sane version is on by default in Clang.