[Cocoa] Some WebKitLegacy headers migrated from WebCore incorrectly contain WEBCORE_EXPORT
Headers migrated from WebCore to WebKitLegacy might contain the WEBCORE_EXPORT macro. Since we don't control the clients of WebKitLegacy, we can't guarantee that this macro will be defined in the translation units including these headers. Previously we've been defining WEBCORE_EXPORT in some of the projects we control, but this isn't a systematic fix. Since WEBCORE_EXPORT meaningless in API headers, let's teach postprocess-headers.sh to strip it out.
Created attachment 237609 [details] Patch
Attachment 237609 [details] did not pass style-queue: ERROR: Source/WebKit2/config.h:33: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 1 in 9 files If any of these errors are false positives, please file a bug against check-webkit-style.
On second thought, it probably makes more sense to do the stripping in MigrateHeaders.make, since that only processes the headers that could contain WEBCORE_EXPORT.
Created attachment 237646 [details] Patch
Attachment 237646 [details] did not pass style-queue: ERROR: Source/WebKit2/config.h:33: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 1 in 9 files If any of these errors are false positives, please file a bug against check-webkit-style.
Committed r173283: <http://trac.webkit.org/changeset/173283>
Comment on attachment 237646 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=237646&action=review I know this patch was already reviewed and landed. I noticed some minor nits. > Source/WebKit/mac/ChangeLog:12 > + This removes WEBCORE_EXPORT and a single following space character but preserves preceeding whitespace. This regular expression doesn't preserve leading whitespace. It preserves leading space characters. I further remark about this in my comment in file Source/WebKit/mac/MigrateHeaders.make. > Source/WebKit/mac/MigrateHeaders.make:240 > +WEBCORE_HEADER_REPLACE_RULES = -e s/\<WebCore/\<WebKitLegacy/ -e s/DOMDOMImplementation/DOMImplementation/ -e "s/(^ *)WEBCORE_EXPORT /\1/" This is OK as-is. Obviously, this regular expression won't match lines that start with tab characters or are a mix of space and tab characters. In theory, we shouldn't have tab characters in source files per the WebKit Code Style Guideline. > Tools/DumpRenderTree/config.h:30 > +#include <WebCore/PlatformExportMacros.h> > #include <wtf/Platform.h> > #include <wtf/ExportMacros.h> > #include <runtime/JSExportMacros.h> These headers aren't listed in alphabetical order. Please sort these headers according to the output of the UNIX sort command. Also, it's unnecessary to explicitly include headers <wtf/Platform.h> and <wtf/ExportMacros.h> as <WebCore/PlatformExportMacros.h> includes them. For completeness, <wtf/ExportMacros.h> is also included by <runtime/JSExportMacros.h>. > Tools/TestWebKitAPI/config.h:33 > +#include <WebCore/PlatformExportMacros.h> > #include <wtf/Platform.h> > #include <wtf/ExportMacros.h> > #include <runtime/JSExportMacros.h> Ditto. > Tools/WebKitTestRunner/config.h:36 > +#include <WebCore/PlatformExportMacros.h> > #include <WebKit/WebKit2_C.h> > #include <wtf/Platform.h> > #include <wtf/ExportMacros.h> > #include <runtime/JSExportMacros.h> Ditto.
(In reply to comment #8) > (From update of attachment 237646 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=237646&action=review > > I know this patch was already reviewed and landed. I noticed some minor nits. > > > Source/WebKit/mac/ChangeLog:12 > > + This removes WEBCORE_EXPORT and a single following space character but preserves preceeding whitespace. > > This regular expression doesn't preserve leading whitespace. It preserves leading space characters. I further remark about this in my comment in file Source/WebKit/mac/MigrateHeaders.make. True, my terminology was imprecise. I only meant to preserve spaces. > > > Source/WebKit/mac/MigrateHeaders.make:240 > > +WEBCORE_HEADER_REPLACE_RULES = -e s/\<WebCore/\<WebKitLegacy/ -e s/DOMDOMImplementation/DOMImplementation/ -e "s/(^ *)WEBCORE_EXPORT /\1/" > > This is OK as-is. Obviously, this regular expression won't match lines that start with tab characters or are a mix of space and tab characters. In theory, we shouldn't have tab characters in source files per the WebKit Code Style Guideline. Right. We shouldn't have tab characters in header files, so I don't account for them. > > > Tools/DumpRenderTree/config.h:30 > > +#include <WebCore/PlatformExportMacros.h> > > #include <wtf/Platform.h> > > #include <wtf/ExportMacros.h> > > #include <runtime/JSExportMacros.h> > > These headers aren't listed in alphabetical order. Please sort these headers according to the output of the UNIX sort command. Oops, I didn't see the existing sorting mistake. Thanks for pointing it out. > > Also, it's unnecessary to explicitly include headers <wtf/Platform.h> and <wtf/ExportMacros.h> as <WebCore/PlatformExportMacros.h> includes them. For completeness, <wtf/ExportMacros.h> is also included by <runtime/JSExportMacros.h>. Ok, I'll clean this up. Thanks Dan!
Addressed Dan's feedback in r173288: <http://trac.webkit.org/changeset/173288>