179893 2017-11-20 12:09:13 -0800 Add declspec within WebKit API 2017-11-20 18:50:25 -0800 1 1 1 Unclassified WebKit WebKit API WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 don.olmstead don.olmstead achristensen bfulgham commit-queue darin rniwa webkit-bug-importer oldest_to_newest 1373811 0 don.olmstead 2017-11-20 12:09:13 -0800 Add __declspec to WebKit API for Windows. 1373812 1 327371 don.olmstead 2017-11-20 12:12:20 -0800 Created attachment 327371 Patch Adds declspec export/import along with a check for clang support of it. 1373824 2 327371 darin 2017-11-20 13:52:25 -0800 Comment on attachment 327371 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=327371&action=review > Source/WebKit/Shared/API/c/WKDeclarationSpecifiers.h:33 > +#if !defined(__has_declspec_attribute) > +#define __has_declspec_attribute(x) 0 > +#endif I suggest putting this in a separate paragraph, not inside the WK_EXPORT paragraph. As is done with __has_extension and __has_attribute below. Also, should use ifndef as all the others do. > Source/WebKit/Shared/API/c/WKDeclarationSpecifiers.h:41 > +#endif /* BUILDING_WEBKIT */ Regardless of the overall merits of this patch, it’d not good to have a comment here that does not match the #if above. The thing about is BUILDING_WebKit, but the comment says BUILDING_WEBKIT. 1373845 3 327377 don.olmstead 2017-11-20 15:10:10 -0800 Created attachment 327377 Patch Updating based on review comments. Adding RealView ARM Compiler support of declspec. 1373863 4 327377 darin 2017-11-20 16:28:34 -0800 Comment on attachment 327377 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=327377&action=review Seems OK to land it like this, although it doesn’t seem exactly right. > Source/WebKit/Shared/API/c/WKDeclarationSpecifiers.h:36 > +#ifndef __has_declspec_attribute > +#if defined(WIN32) || defined(_WIN32) || defined(__CC_ARM) || defined(__ARMCC__) > +#define __has_declspec_attribute(x) 1 > +#else > +#define __has_declspec_attribute(x) 0 > +#endif > +#endif This doesn’t seem quite right to me; it’s not great that __has_declspec_attribute(THIS_DECLSPEC_ATTRIBUTE_IS_BOGUS_AND_DOES_NOT_EXIST) will return true on many platforms. 1373871 5 don.olmstead 2017-11-20 16:46:43 -0800 (In reply to Darin Adler from comment #4) > Comment on attachment 327377 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=327377&action=review > > Seems OK to land it like this, although it doesn’t seem exactly right. > > > Source/WebKit/Shared/API/c/WKDeclarationSpecifiers.h:36 > > +#ifndef __has_declspec_attribute > > +#if defined(WIN32) || defined(_WIN32) || defined(__CC_ARM) || defined(__ARMCC__) > > +#define __has_declspec_attribute(x) 1 > > +#else > > +#define __has_declspec_attribute(x) 0 > > +#endif > > +#endif > > This doesn’t seem quite right to me; it’s not great that > __has_declspec_attribute(THIS_DECLSPEC_ATTRIBUTE_IS_BOGUS_AND_DOES_NOT_EXIST) > will return true on many platforms. Would you prefer it to do #ifndef __has_declspec_attribute #define __has_declspec_attribute(x) 0 #endif then use it like #elif defined(WIN32) || defined(_WIN32) || defined(__CC_ARM) || defined(__ARMCC__) || (__has_declspec_attribute(dllimport) && __has_declspec_attribute(dllexport)) 1373878 6 darin 2017-11-20 17:08:03 -0800 (In reply to Don Olmstead from comment #5) > Would you prefer it to do > > #ifndef __has_declspec_attribute > #define __has_declspec_attribute(x) 0 > #endif > > then use it like > > #elif defined(WIN32) || defined(_WIN32) || defined(__CC_ARM) || > defined(__ARMCC__) || (__has_declspec_attribute(dllimport) && > __has_declspec_attribute(dllexport)) I do think that’s slightly better. 1373880 7 327379 don.olmstead 2017-11-20 17:18:21 -0800 Created attachment 327379 Patch Addressing feedback 1373896 8 327379 commit-queue 2017-11-20 18:49:39 -0800 Comment on attachment 327379 Patch Clearing flags on attachment: 327379 Committed r225064: <https://trac.webkit.org/changeset/225064> 1373897 9 commit-queue 2017-11-20 18:49:40 -0800 All reviewed patches have been landed. Closing bug. 1373898 10 webkit-bug-importer 2017-11-20 18:50:25 -0800 <rdar://problem/35651307> 327371 2017-11-20 12:12:20 -0800 2017-11-20 15:10:10 -0800 Patch bug-179893.diff text/plain 1474 don.olmstead ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nIGIvU291cmNlL1dlYktpdC9DaGFu Z2VMb2cKaW5kZXggODhjNzczMTA2ZTQuLmQ5YmQxYmEyYWIzIDEwMDY0NAotLS0gYS9Tb3VyY2Uv V2ViS2l0L0NoYW5nZUxvZworKysgYi9Tb3VyY2UvV2ViS2l0L0NoYW5nZUxvZwpAQCAtMSwzICsx LDEyIEBACisyMDE3LTExLTIwICBEb24gT2xtc3RlYWQgIDxkb24ub2xtc3RlYWRAc29ueS5jb20+ CisKKyAgICAgICAgQWRkIGRlY2xzcGVjIHdpdGhpbiBXZWJLaXQgQVBJCisgICAgICAgIGh0dHBz Oi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xNzk4OTMKKworICAgICAgICBSZXZp ZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICAqIFNoYXJlZC9BUEkvYy9XS0RlY2xh cmF0aW9uU3BlY2lmaWVycy5oOgorCiAyMDE3LTExLTIwICBDYXJsb3MgR2FyY2lhIENhbXBvcyAg PGNnYXJjaWFAaWdhbGlhLmNvbT4KIAogICAgICAgICBbV1BFXSB3ZWJraXRfd2ViX3ZpZXdfbmV3 KCkgc2hvdWxkIGVuYWJsZSBzcGVjaWZ5aW5nIHdwZV92aWV3X2JhY2tlbmQgb2JqZWN0CmRpZmYg LS1naXQgYS9Tb3VyY2UvV2ViS2l0L1NoYXJlZC9BUEkvYy9XS0RlY2xhcmF0aW9uU3BlY2lmaWVy cy5oIGIvU291cmNlL1dlYktpdC9TaGFyZWQvQVBJL2MvV0tEZWNsYXJhdGlvblNwZWNpZmllcnMu aAppbmRleCBhMDQ3NDQ3MzlmMy4uOTZiMzVlMjdkYjEgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJL aXQvU2hhcmVkL0FQSS9jL1dLRGVjbGFyYXRpb25TcGVjaWZpZXJzLmgKKysrIGIvU291cmNlL1dl YktpdC9TaGFyZWQvQVBJL2MvV0tEZWNsYXJhdGlvblNwZWNpZmllcnMuaApAQCAtMjgsOCArMjgs MTcgQEAKICNkZWZpbmUgV0tEZWNsYXJhdGlvblNwZWNpZmllcnNfaAogCiAjdW5kZWYgV0tfRVhQ T1JUCisjaWYgIWRlZmluZWQoX19oYXNfZGVjbHNwZWNfYXR0cmlidXRlKQorI2RlZmluZSBfX2hh c19kZWNsc3BlY19hdHRyaWJ1dGUoeCkgMAorI2VuZGlmCiAjaWYgZGVmaW5lZChXS19OT19FWFBP UlQpCiAjZGVmaW5lIFdLX0VYUE9SVAorI2VsaWYgZGVmaW5lZChXSU4zMikgfHwgZGVmaW5lZChf V0lOMzIpIHx8IChfX2hhc19kZWNsc3BlY19hdHRyaWJ1dGUoZGxsaW1wb3J0KSAmJiBfX2hhc19k ZWNsc3BlY19hdHRyaWJ1dGUoZGxsZXhwb3J0KSkKKyNpZiBCVUlMRElOR19XZWJLaXQKKyNkZWZp bmUgV0tfRVhQT1JUIF9fZGVjbHNwZWMoZGxsZXhwb3J0KQorI2Vsc2UKKyNkZWZpbmUgV0tfRVhQ T1JUIF9fZGVjbHNwZWMoZGxsaW1wb3J0KQorI2VuZGlmIC8qIEJVSUxESU5HX1dFQktJVCAqLwog I2VsaWYgZGVmaW5lZChfX0dOVUNfXykgJiYgIWRlZmluZWQoX19DQ19BUk0pICYmICFkZWZpbmVk KF9fQVJNQ0NfXykKICNkZWZpbmUgV0tfRVhQT1JUIF9fYXR0cmlidXRlX18oKHZpc2liaWxpdHko ImRlZmF1bHQiKSkpCiAjZWxzZSAvKiAhZGVmaW5lZChXS19OT19FWFBPUlQpICovCg== 327377 2017-11-20 15:10:10 -0800 2017-11-20 17:18:21 -0800 Patch bug-179893.diff text/plain 1647 don.olmstead ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nIGIvU291cmNlL1dlYktpdC9DaGFu Z2VMb2cKaW5kZXggODhjNzczMTA2ZTQuLmQ5YmQxYmEyYWIzIDEwMDY0NAotLS0gYS9Tb3VyY2Uv V2ViS2l0L0NoYW5nZUxvZworKysgYi9Tb3VyY2UvV2ViS2l0L0NoYW5nZUxvZwpAQCAtMSwzICsx LDEyIEBACisyMDE3LTExLTIwICBEb24gT2xtc3RlYWQgIDxkb24ub2xtc3RlYWRAc29ueS5jb20+ CisKKyAgICAgICAgQWRkIGRlY2xzcGVjIHdpdGhpbiBXZWJLaXQgQVBJCisgICAgICAgIGh0dHBz Oi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xNzk4OTMKKworICAgICAgICBSZXZp ZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICAqIFNoYXJlZC9BUEkvYy9XS0RlY2xh cmF0aW9uU3BlY2lmaWVycy5oOgorCiAyMDE3LTExLTIwICBDYXJsb3MgR2FyY2lhIENhbXBvcyAg PGNnYXJjaWFAaWdhbGlhLmNvbT4KIAogICAgICAgICBbV1BFXSB3ZWJraXRfd2ViX3ZpZXdfbmV3 KCkgc2hvdWxkIGVuYWJsZSBzcGVjaWZ5aW5nIHdwZV92aWV3X2JhY2tlbmQgb2JqZWN0CmRpZmYg LS1naXQgYS9Tb3VyY2UvV2ViS2l0L1NoYXJlZC9BUEkvYy9XS0RlY2xhcmF0aW9uU3BlY2lmaWVy cy5oIGIvU291cmNlL1dlYktpdC9TaGFyZWQvQVBJL2MvV0tEZWNsYXJhdGlvblNwZWNpZmllcnMu aAppbmRleCBhMDQ3NDQ3MzlmMy4uNmFmM2EyNzkwM2EgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJL aXQvU2hhcmVkL0FQSS9jL1dLRGVjbGFyYXRpb25TcGVjaWZpZXJzLmgKKysrIGIvU291cmNlL1dl YktpdC9TaGFyZWQvQVBJL2MvV0tEZWNsYXJhdGlvblNwZWNpZmllcnMuaApAQCAtMjcsMTAgKzI3 LDI0IEBACiAjaWZuZGVmIFdLRGVjbGFyYXRpb25TcGVjaWZpZXJzX2gKICNkZWZpbmUgV0tEZWNs YXJhdGlvblNwZWNpZmllcnNfaAogCisjaWZuZGVmIF9faGFzX2RlY2xzcGVjX2F0dHJpYnV0ZQor I2lmIGRlZmluZWQoV0lOMzIpIHx8IGRlZmluZWQoX1dJTjMyKSB8fCBkZWZpbmVkKF9fQ0NfQVJN KSB8fCBkZWZpbmVkKF9fQVJNQ0NfXykKKyNkZWZpbmUgX19oYXNfZGVjbHNwZWNfYXR0cmlidXRl KHgpIDEKKyNlbHNlCisjZGVmaW5lIF9faGFzX2RlY2xzcGVjX2F0dHJpYnV0ZSh4KSAwCisjZW5k aWYKKyNlbmRpZgorCiAjdW5kZWYgV0tfRVhQT1JUCiAjaWYgZGVmaW5lZChXS19OT19FWFBPUlQp CiAjZGVmaW5lIFdLX0VYUE9SVAotI2VsaWYgZGVmaW5lZChfX0dOVUNfXykgJiYgIWRlZmluZWQo X19DQ19BUk0pICYmICFkZWZpbmVkKF9fQVJNQ0NfXykKKyNlbGlmIF9faGFzX2RlY2xzcGVjX2F0 dHJpYnV0ZShkbGxpbXBvcnQpICYmIF9faGFzX2RlY2xzcGVjX2F0dHJpYnV0ZShkbGxleHBvcnQp CisjaWYgQlVJTERJTkdfV2ViS2l0CisjZGVmaW5lIFdLX0VYUE9SVCBfX2RlY2xzcGVjKGRsbGV4 cG9ydCkKKyNlbHNlCisjZGVmaW5lIFdLX0VYUE9SVCBfX2RlY2xzcGVjKGRsbGltcG9ydCkKKyNl bmRpZiAvKiBCVUlMRElOR19XZWJLaXQgKi8KKyNlbGlmIGRlZmluZWQoX19HTlVDX18pCiAjZGVm aW5lIFdLX0VYUE9SVCBfX2F0dHJpYnV0ZV9fKCh2aXNpYmlsaXR5KCJkZWZhdWx0IikpKQogI2Vs c2UgLyogIWRlZmluZWQoV0tfTk9fRVhQT1JUKSAqLwogI2RlZmluZSBXS19FWFBPUlQK 327379 2017-11-20 17:18:21 -0800 2017-11-20 18:49:39 -0800 Patch bug-179893.diff text/plain 1586 don.olmstead ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nIGIvU291cmNlL1dlYktpdC9DaGFu Z2VMb2cKaW5kZXggMWY1MzJhZjNhZGEuLjE4MTA3Y2ZjZDE0IDEwMDY0NAotLS0gYS9Tb3VyY2Uv V2ViS2l0L0NoYW5nZUxvZworKysgYi9Tb3VyY2UvV2ViS2l0L0NoYW5nZUxvZwpAQCAtMSwzICsx LDEyIEBACisyMDE3LTExLTIwICBEb24gT2xtc3RlYWQgIDxkb24ub2xtc3RlYWRAc29ueS5jb20+ CisKKyAgICAgICAgQWRkIGRlY2xzcGVjIHdpdGhpbiBXZWJLaXQgQVBJCisgICAgICAgIGh0dHBz Oi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xNzk4OTMKKworICAgICAgICBSZXZp ZXdlZCBieSBEYXJpbiBBZGxlci4KKworICAgICAgICAqIFNoYXJlZC9BUEkvYy9XS0RlY2xhcmF0 aW9uU3BlY2lmaWVycy5oOgorCiAyMDE3LTExLTIwICBNYXJpbyBTYW5jaGV6IFByYWRhICA8bWFy aW9AZW5kbGVzc20uY29tPgogCiAgICAgICAgIFtHVEtdIE5ldyBBUEkgdG8gYWRkLCByZXRyaWV2 ZSBhbmQgZGVsZXRlIGNvb2tpZXMgdmlhIFdlYktpdENvb2tpZU1hbmFnZXIKZGlmZiAtLWdpdCBh L1NvdXJjZS9XZWJLaXQvU2hhcmVkL0FQSS9jL1dLRGVjbGFyYXRpb25TcGVjaWZpZXJzLmggYi9T b3VyY2UvV2ViS2l0L1NoYXJlZC9BUEkvYy9XS0RlY2xhcmF0aW9uU3BlY2lmaWVycy5oCmluZGV4 IGEwNDc0NDczOWYzLi5kNWUzMTk4ODU3ZSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYktpdC9TaGFy ZWQvQVBJL2MvV0tEZWNsYXJhdGlvblNwZWNpZmllcnMuaAorKysgYi9Tb3VyY2UvV2ViS2l0L1No YXJlZC9BUEkvYy9XS0RlY2xhcmF0aW9uU3BlY2lmaWVycy5oCkBAIC0yNywxMCArMjcsMjAgQEAK ICNpZm5kZWYgV0tEZWNsYXJhdGlvblNwZWNpZmllcnNfaAogI2RlZmluZSBXS0RlY2xhcmF0aW9u U3BlY2lmaWVyc19oCiAKKyNpZm5kZWYgX19oYXNfZGVjbHNwZWNfYXR0cmlidXRlCisjZGVmaW5l IF9faGFzX2RlY2xzcGVjX2F0dHJpYnV0ZSh4KSAwCisjZW5kaWYKKwogI3VuZGVmIFdLX0VYUE9S VAogI2lmIGRlZmluZWQoV0tfTk9fRVhQT1JUKQogI2RlZmluZSBXS19FWFBPUlQKLSNlbGlmIGRl ZmluZWQoX19HTlVDX18pICYmICFkZWZpbmVkKF9fQ0NfQVJNKSAmJiAhZGVmaW5lZChfX0FSTUND X18pCisjZWxpZiBkZWZpbmVkKFdJTjMyKSB8fCBkZWZpbmVkKF9XSU4zMikgfHwgZGVmaW5lZChf X0NDX0FSTSkgfHwgZGVmaW5lZChfX0FSTUNDX18pIHx8IChfX2hhc19kZWNsc3BlY19hdHRyaWJ1 dGUoZGxsaW1wb3J0KSAmJiBfX2hhc19kZWNsc3BlY19hdHRyaWJ1dGUoZGxsZXhwb3J0KSkKKyNp ZiBCVUlMRElOR19XZWJLaXQKKyNkZWZpbmUgV0tfRVhQT1JUIF9fZGVjbHNwZWMoZGxsZXhwb3J0 KQorI2Vsc2UKKyNkZWZpbmUgV0tfRVhQT1JUIF9fZGVjbHNwZWMoZGxsaW1wb3J0KQorI2VuZGlm IC8qIEJVSUxESU5HX1dlYktpdCAqLworI2VsaWYgZGVmaW5lZChfX0dOVUNfXykKICNkZWZpbmUg V0tfRVhQT1JUIF9fYXR0cmlidXRlX18oKHZpc2liaWxpdHkoImRlZmF1bHQiKSkpCiAjZWxzZSAv KiAhZGVmaW5lZChXS19OT19FWFBPUlQpICovCiAjZGVmaW5lIFdLX0VYUE9SVAo=