171240 2017-04-24 12:07:47 -0700 [Win] Use Clang's __has_declspec_attribute for export macros 2017-04-25 14:46:59 -0700 1 1 1 Unclassified WebKit Tools / Tests WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 don.olmstead webkit-unassigned achristensen benjamin buildbot cdumez cmarcelo commit-queue dbates fujii keith_miller lforschler mark.lam msaboff saam oldest_to_newest 1300599 0 don.olmstead 2017-04-24 12:07:47 -0700 Currently a OS(WINDOWS) check is used to determine whether __declspec should be used for exporting symbols in shared libraries. Clang defines a macro __has_declspec_attribute that can be used to determine if the platform supports __declspec. 1300601 1 307995 don.olmstead 2017-04-24 12:10:42 -0700 Created attachment 307995 Patch 1300602 2 buildbot 2017-04-24 12:13:02 -0700 Attachment 307995 did not pass style-queue: ERROR: Source/WTF/wtf/Compiler.h:54: One space before end of line comments [whitespace/comments] [5] ERROR: Source/WTF/wtf/Compiler.h:54: Should have a space between // and comment [whitespace/comments] [4] Total errors found: 2 in 6 files If any of these errors are false positives, please file a bug against check-webkit-style. 1300702 3 307995 achristensen 2017-04-24 16:25:16 -0700 Comment on attachment 307995 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=307995&action=review > Source/WTF/wtf/Platform.h:440 > +#if OS(WINDOWS) \ > + || (COMPILER_HAS_CLANG_DECLSPEC(dllimport) && COMPILER_HAS_CLANG_DECLSPEC(dllexport)) These can be on one line. Also, if it has __declspec(dllimport) can't we assume it has __declspec(dllexport)? > Source/WTF/wtf/Platform.h:441 > +#define USE_DECLSPEC_ATTRIBUTE 1 __declspec is a keyword according to https://msdn.microsoft.com/en-us/library/dabb5z75.aspx I wish clang had used __has_declspec_keyword, but they didn't. > Source/WTF/wtf/Platform.h:443 > +#elif defined(__GNUC__) && !defined(__CC_ARM) && !defined(__ARMCC__) Is there no visibility attribute on ARM? 1301116 4 don.olmstead 2017-04-25 11:01:15 -0700 (In reply to Alex Christensen from comment #3) > Comment on attachment 307995 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=307995&action=review > > > Source/WTF/wtf/Platform.h:440 > > +#if OS(WINDOWS) \ > > + || (COMPILER_HAS_CLANG_DECLSPEC(dllimport) && COMPILER_HAS_CLANG_DECLSPEC(dllexport)) > > These can be on one line. > Also, if it has __declspec(dllimport) can't we assume it has > __declspec(dllexport)? I would assume that would be the case but just made it completely explicit. Can change that. > > > Source/WTF/wtf/Platform.h:441 > > +#define USE_DECLSPEC_ATTRIBUTE 1 > > __declspec is a keyword according to > https://msdn.microsoft.com/en-us/library/dabb5z75.aspx > I wish clang had used __has_declspec_keyword, but they didn't. > > > Source/WTF/wtf/Platform.h:443 > > +#elif defined(__GNUC__) && !defined(__CC_ARM) && !defined(__ARMCC__) > > Is there no visibility attribute on ARM? That line has actually been there since the first commit of ExportMacros.h so its over 6 years old. I'm not sure the initial thinking of it. Should we just remove it? 1301227 5 307995 achristensen 2017-04-25 14:14:07 -0700 Comment on attachment 307995 Patch Let's just do this as it is now. 1301259 6 307995 commit-queue 2017-04-25 14:46:57 -0700 Comment on attachment 307995 Patch Clearing flags on attachment: 307995 Committed r215766: <http://trac.webkit.org/changeset/215766> 1301260 7 commit-queue 2017-04-25 14:46:59 -0700 All reviewed patches have been landed. Closing bug. 307995 2017-04-24 12:10:42 -0700 2017-04-25 14:46:57 -0700 Patch bug171240.diff text/plain 4694 don.olmstead ZGlmZiAtLWdpdCBhL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cgYi9Tb3VyY2UvSmF2 YVNjcmlwdENvcmUvQ2hhbmdlTG9nCmluZGV4IDRkMjFkNDEuLjhjZWQzYjQgMTAwNjQ0Ci0tLSBh L1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL0phdmFTY3JpcHRD b3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDEyIEBACisyMDE3LTA0LTI0ICBEb24gT2xtc3RlYWQg IDxkb24ub2xtc3RlYWRAYW0uc29ueS5jb20+CisKKyAgICAgICAgW1dpbl0gVXNlIENsYW5nJ3Mg X19oYXNfZGVjbHNwZWNfYXR0cmlidXRlIGZvciBleHBvcnQgbWFjcm9zCisgICAgICAgIGh0dHBz Oi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xNzEyNDAKKworICAgICAgICBSZXZp ZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICAqIHJ1bnRpbWUvSlNFeHBvcnRNYWNy b3MuaDoKKwogMjAxNy0wNC0yNCAgSm9zZXBoIFBlY29yYXJvICA8cGVjb3Jhcm9AYXBwbGUuY29t PgogCiAgICAgICAgIHRlc3QyNjI6IHRlc3QyNjIvdGVzdC9sYW5ndWFnZS9zdGF0ZW1lbnRzL2Zv ci1vZi9kc3RyLWFycmF5LWVsZW0taW5pdC1mbi1uYW1lLWFycm93LmpzCmRpZmYgLS1naXQgYS9T b3VyY2UvSmF2YVNjcmlwdENvcmUvcnVudGltZS9KU0V4cG9ydE1hY3Jvcy5oIGIvU291cmNlL0ph dmFTY3JpcHRDb3JlL3J1bnRpbWUvSlNFeHBvcnRNYWNyb3MuaAppbmRleCAxMjZjYmM5Li5jYTMx Mzg5IDEwMDY0NAotLS0gYS9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvcnVudGltZS9KU0V4cG9ydE1h Y3Jvcy5oCisrKyBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9ydW50aW1lL0pTRXhwb3J0TWFjcm9z LmgKQEAgLTQ2LDcgKzQ2LDcgQEAKIAogI2Vsc2UgLy8gIVVTRShFWFBPUlRfTUFDUk9TKQogCi0j aWYgT1MoV0lORE9XUykgJiYgIUNPTVBJTEVSKEdDQ19PUl9DTEFORykKKyNpZiBVU0UoREVDTFNQ RUNfQVRUUklCVVRFKQogCiAjaWYgZGVmaW5lZChCVUlMRElOR19KYXZhU2NyaXB0Q29yZSkgfHwg ZGVmaW5lZChTVEFUSUNBTExZX0xJTktFRF9XSVRIX0phdmFTY3JpcHRDb3JlKQogI2RlZmluZSBK U19FWFBPUlREQVRBIF9fZGVjbHNwZWMoZGxsZXhwb3J0KQpkaWZmIC0tZ2l0IGEvU291cmNlL1dU Ri9DaGFuZ2VMb2cgYi9Tb3VyY2UvV1RGL0NoYW5nZUxvZwppbmRleCA2YWRhNTU0Li5hNmIyNmU5 IDEwMDY0NAotLS0gYS9Tb3VyY2UvV1RGL0NoYW5nZUxvZworKysgYi9Tb3VyY2UvV1RGL0NoYW5n ZUxvZwpAQCAtMSwzICsxLDE0IEBACisyMDE3LTA0LTI0ICBEb24gT2xtc3RlYWQgIDxkb24ub2xt c3RlYWRAYW0uc29ueS5jb20+CisKKyAgICAgICAgW1dpbl0gVXNlIENsYW5nJ3MgX19oYXNfZGVj bHNwZWNfYXR0cmlidXRlIGZvciBleHBvcnQgbWFjcm9zCisgICAgICAgIGh0dHBzOi8vYnVncy53 ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xNzEyNDAKKworICAgICAgICBSZXZpZXdlZCBieSBO T0JPRFkgKE9PUFMhKS4KKworICAgICAgICAqIHd0Zi9Db21waWxlci5oOgorICAgICAgICAqIHd0 Zi9FeHBvcnRNYWNyb3MuaDoKKyAgICAgICAgKiB3dGYvUGxhdGZvcm0uaDoKKwogMjAxNy0wNC0y NCAgQ2FybG9zIEdhcmNpYSBDYW1wb3MgIDxjZ2FyY2lhQGlnYWxpYS5jb20+CiAKICAgICAgICAg W0dUS10gU3dpdGNoIHRvIHVzZSBFTkFCTEVfUkVNT1RFX0lOU1BFQ1RPUiBpbnN0ZWFkIG9mIEVO QUJMRV9JTlNQRUNUT1JfU0VSVkVSIGZvciB0aGUgcmVtb3RlIGluc3BlY3RvcgpkaWZmIC0tZ2l0 IGEvU291cmNlL1dURi93dGYvQ29tcGlsZXIuaCBiL1NvdXJjZS9XVEYvd3RmL0NvbXBpbGVyLmgK aW5kZXggZDY1MTNhOS4uOTcyMDc1ZSAxMDA2NDQKLS0tIGEvU291cmNlL1dURi93dGYvQ29tcGls ZXIuaAorKysgYi9Tb3VyY2UvV1RGL3d0Zi9Db21waWxlci5oCkBAIC01MCw2ICs1MCwxNCBAQAog I2RlZmluZSBDT01QSUxFUl9IQVNfQ0xBTkdfRkVBVFVSRSh4KSAwCiAjZW5kaWYKIAorLyogQ09N UElMRVJfSEFTX0NMQU5HX0RFQ0xTUEVDKCkgLSB3aGV0aGVyIHRoZSBjb21waWxlciBzdXBwb3J0 cyBhIE1pY3Jvc29mdCBzdHlsZSBfX2RlY2xzcGVjIGF0dHJpYnV0ZS4gKi8KKy8qIGh0dHBzOi8v Y2xhbmcubGx2bS5vcmcvZG9jcy9MYW5ndWFnZUV4dGVuc2lvbnMuaHRtbCNoYXMtZGVjbHNwZWMt YXR0cmlidXRlICovCisjaWZkZWYgX19oYXNfZGVjbHNwZWNfYXR0cmlidXRlCisjZGVmaW5lIENP TVBJTEVSX0hBU19DTEFOR19ERUNMU1BFQyh4KSBfX2hhc19kZWNsc3BlY19hdHRyaWJ1dGUoeCkK KyNlbHNlCisjZGVmaW5lIENPTVBJTEVSX0hBU19DTEFOR19ERUNMU1BFQyh4KSAwCisjZW5kaWYK KwogLyogPT09PSBDT01QSUxFUigpIC0gcHJpbWFyeSBkZXRlY3Rpb24gb2YgdGhlIGNvbXBpbGVy IGJlaW5nIHVzZWQgdG8gYnVpbGQgdGhlIHByb2plY3QsIGluIGFscGhhYmV0aWNhbCBvcmRlciA9 PT09ICovCiAKIC8qIENPTVBJTEVSKENMQU5HKSAtIENsYW5nICAqLwpkaWZmIC0tZ2l0IGEvU291 cmNlL1dURi93dGYvRXhwb3J0TWFjcm9zLmggYi9Tb3VyY2UvV1RGL3d0Zi9FeHBvcnRNYWNyb3Mu aAppbmRleCA1YWU4N2QwLi5lODk5YjQ0IDEwMDY0NAotLS0gYS9Tb3VyY2UvV1RGL3d0Zi9FeHBv cnRNYWNyb3MuaAorKysgYi9Tb3VyY2UvV1RGL3d0Zi9FeHBvcnRNYWNyb3MuaApAQCAtMzgsMjMg KzM4LDIzIEBACiAvLyBiZWluZyBsb2NhbCB0byB0aGUgdGFyZ2V0IGJlaW5nIGdlbmVyYXRlZCwg YW5kIHRodXMgbm90IHN1YmplY3QgdG8gKGUuZy4pIEVMRgogLy8gc3ltYm9sIGludGVycG9zaXRp b24gcnVsZXMuCiAKLSNpZiBPUyhXSU5ET1dTKQorI2lmIFVTRShERUNMU1BFQ19BVFRSSUJVVEUp CiAjZGVmaW5lIEhBVkVfSU5URVJOQUxfVklTSUJJTElUWSAxCiAjZGVmaW5lIFdURl9JTlRFUk5B TAotI2VsaWYgZGVmaW5lZChfX0dOVUNfXykgJiYgIWRlZmluZWQoX19DQ19BUk0pICYmICFkZWZp bmVkKF9fQVJNQ0NfXykKKyNlbGlmIFVTRShWSVNJQklMSVRZX0FUVFJJQlVURSkKICNkZWZpbmUg SEFWRV9JTlRFUk5BTF9WSVNJQklMSVRZIDEKICNkZWZpbmUgV1RGX0lOVEVSTkFMIF9fYXR0cmli dXRlX18oKHZpc2liaWxpdHkoImhpZGRlbiIpKSkKICNlbHNlCiAjZGVmaW5lIFdURl9JTlRFUk5B TAogI2VuZGlmCiAKLSNpZiBPUyhXSU5ET1dTKQorI2lmIFVTRShERUNMU1BFQ19BVFRSSUJVVEUp CiAKICNkZWZpbmUgV1RGX0VYUE9SVF9ERUNMQVJBVElPTiBfX2RlY2xzcGVjKGRsbGV4cG9ydCkK ICNkZWZpbmUgV1RGX0lNUE9SVF9ERUNMQVJBVElPTiBfX2RlY2xzcGVjKGRsbGltcG9ydCkKICNk ZWZpbmUgV1RGX0hJRERFTl9ERUNMQVJBVElPTgogCi0jZWxpZiBkZWZpbmVkKF9fR05VQ19fKSAm JiAhZGVmaW5lZChfX0NDX0FSTSkgJiYgIWRlZmluZWQoX19BUk1DQ19fKQorI2VsaWYgVVNFKFZJ U0lCSUxJVFlfQVRUUklCVVRFKQogCiAjZGVmaW5lIFdURl9FWFBPUlRfREVDTEFSQVRJT04gX19h dHRyaWJ1dGVfXygodmlzaWJpbGl0eSgiZGVmYXVsdCIpKSkKICNkZWZpbmUgV1RGX0lNUE9SVF9E RUNMQVJBVElPTiBXVEZfRVhQT1JUX0RFQ0xBUkFUSU9OCkBAIC04OSw3ICs4OSw3IEBACiAKICNl bHNlIC8vICFVU0UoRVhQT1JUX01BQ1JPUykKIAotI2lmIE9TKFdJTkRPV1MpICYmICFDT01QSUxF UihHQ0NfT1JfQ0xBTkcpCisjaWYgVVNFKERFQ0xTUEVDX0FUVFJJQlVURSkKICNpZiBkZWZpbmVk KEJVSUxESU5HX1dURikgfHwgZGVmaW5lZChTVEFUSUNBTExZX0xJTktFRF9XSVRIX1dURikKICNk ZWZpbmUgV1RGX0VYUE9SVERBVEEgX19kZWNsc3BlYyhkbGxleHBvcnQpCiAjZWxzZQpkaWZmIC0t Z2l0IGEvU291cmNlL1dURi93dGYvUGxhdGZvcm0uaCBiL1NvdXJjZS9XVEYvd3RmL1BsYXRmb3Jt LmgKaW5kZXggZThjZjdiNC4uYTc5YjMwZiAxMDA2NDQKLS0tIGEvU291cmNlL1dURi93dGYvUGxh dGZvcm0uaAorKysgYi9Tb3VyY2UvV1RGL3d0Zi9QbGF0Zm9ybS5oCkBAIC00MzQsNiArNDM0LDIw IEBACiAKIC8qIE9wZXJhdGluZyBlbnZpcm9ubWVudHMgKi8KIAorLyogRXhwb3J0IG1hY3JvIHN1 cHBvcnQuIERldGVjdHMgdGhlIGF0dHJpYnV0ZXMgYXZhaWxhYmxlIGZvciBzaGFyZWQgbGlicmFy eSBzeW1ib2wgZXhwb3J0CisgICBkZWNvcmF0aW9ucy4gKi8KKyNpZiBPUyhXSU5ET1dTKSBcCisg ICAgfHwgKENPTVBJTEVSX0hBU19DTEFOR19ERUNMU1BFQyhkbGxpbXBvcnQpICYmIENPTVBJTEVS X0hBU19DTEFOR19ERUNMU1BFQyhkbGxleHBvcnQpKQorI2RlZmluZSBVU0VfREVDTFNQRUNfQVRU UklCVVRFIDEKKyNkZWZpbmUgVVNFX1ZJU0lCSUxJVFlfQVRUUklCVVRFIDAKKyNlbGlmIGRlZmlu ZWQoX19HTlVDX18pICYmICFkZWZpbmVkKF9fQ0NfQVJNKSAmJiAhZGVmaW5lZChfX0FSTUNDX18p CisjZGVmaW5lIFVTRV9ERUNMU1BFQ19BVFRSSUJVVEUgMAorI2RlZmluZSBVU0VfVklTSUJJTElU WV9BVFRSSUJVVEUgMQorI2Vsc2UKKyNkZWZpbmUgVVNFX0RFQ0xTUEVDX0FUVFJJQlVURSAwCisj ZGVmaW5lIFVTRV9WSVNJQklMSVRZX0FUVFJJQlVURSAwCisjZW5kaWYKKwogLyogU3RhbmRhcmQg bGlicmFyaWVzICovCiAjaWYgZGVmaW5lZChIQVZFX0ZFQVRVUkVTX0gpICYmIEhBVkVfRkVBVFVS RVNfSAogLyogSWYgdGhlIGluY2x1ZGVkIGZlYXR1cmVzLmggaXMgZ2xpYmMncyBvbmUsIF9fR0xJ QkNfXyBpcyBkZWZpbmVkLiAqLwo=