Summary: | Turn on overloaded virtual functions warning in WebCore | ||
---|---|---|---|
Product: | WebKit | Reporter: | Simon Fraser (smfr) <simon.fraser> |
Component: | WebCore Misc. | Assignee: | Nobody <webkit-unassigned> |
Status: | RESOLVED WONTFIX | ||
Severity: | Normal | CC: | andersca, darin, sam, zwarich |
Priority: | P2 | ||
Version: | 528+ (Nightly build) | ||
Hardware: | Mac | ||
OS: | OS X 10.5 | ||
Bug Depends on: | |||
Bug Blocks: | 23352 |
Description
Simon Fraser (smfr)
2009-01-05 11:44:06 PST
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; 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. The first issue I hit in WebCore was: virtual void imageChanged(CachedImage*, const IntRect* = 0); virtual void imageChanged(WrappedImagePtr, const IntRect* = 0) { } (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. 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 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. I'm just going to close this, because the GCC warning is broken and the sane version is on by default in Clang. |