Summary: | Refactoring: Rename ScrambledPtr to Poisoned. | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Mark Lam <mark.lam> | ||||||||||||
Component: | JavaScriptCore | Assignee: | Mark Lam <mark.lam> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | benjamin, cdumez, cmarcelo, dbates, ews-watchlist, fpizlo, jfbastien, keith_miller, mcatanzaro, msaboff, rmorisset, saam, webkit-bug-importer | ||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||
Hardware: | Unspecified | ||||||||||||||
OS: | Unspecified | ||||||||||||||
Attachments: |
|
Description
Mark Lam
2017-12-06 17:29:48 PST
Created attachment 328665 [details]
proposed patch.
Attachment 328665 [details] did not pass style-queue:
ERROR: Source/JavaScriptCore/runtime/JSCPoisonedPtr.cpp:31: g_classInfoPoison is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/runtime/JSCPoisonedPtr.cpp:32: g_masmPoison is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4]
Total errors found: 2 in 26 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 328669 [details]
patch for landing + cmake build fix.
Attachment 328669 [details] did not pass style-queue:
ERROR: Source/JavaScriptCore/runtime/JSCPoisonedPtr.cpp:31: g_classInfoPoison is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/runtime/JSCPoisonedPtr.cpp:32: g_masmPoison is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4]
Total errors found: 2 in 27 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 328672 [details]
patch for landing + speculative fix for Windows.
Attachment 328672 [details] did not pass style-queue:
ERROR: Source/JavaScriptCore/runtime/JSCPoisonedPtr.cpp:31: g_classInfoPoison is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/runtime/JSCPoisonedPtr.cpp:32: g_masmPoison is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4]
Total errors found: 2 in 27 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 328677 [details]
patch for landing + 32-bit windows build fix.
Attachment 328677 [details] did not pass style-queue:
ERROR: Source/JavaScriptCore/runtime/JSCPoisonedPtr.cpp:31: g_classInfoPoison is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/runtime/JSCPoisonedPtr.cpp:32: g_masmPoison is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4]
Total errors found: 2 in 27 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Thanks for the review. Landed in r225620: <http://trac.webkit.org/r225620>. Comment on attachment 328677 [details] patch for landing + 32-bit windows build fix. View in context: https://bugs.webkit.org/attachment.cgi?id=328677&action=review > Source/WTF/wtf/Poisoned.h:44 > -template<typename T, uintptr_t& key, typename = std::enable_if_t<std::is_pointer<T>::value>> > -class ScrambledPtr { > +template<typename KeyType, KeyType key, typename T, typename = std::enable_if_t<std::is_pointer<T>::value>> > +class PoisonedImpl { Adding KeyType to the template parameters has broken the build with GCC 7. I don't know how to fix it, so I'm going to roll it out... sorry. I guess GCC 6 works, because EWS and all the bots are fine. In file included from ../../Source/JavaScriptCore/runtime/JSCPoisonedPtr.h:28:0, from ../../Source/JavaScriptCore/assembler/MacroAssemblerCodeRef.h:29, from ../../Source/JavaScriptCore/interpreter/AbstractPC.h:28, from ../../Source/JavaScriptCore/interpreter/CallFrame.h:25, from ../../Source/JavaScriptCore/runtime/ClassInfo.h:25, from ../../Source/JavaScriptCore/runtime/Structure.h:28, from ../../Source/JavaScriptCore/bytecode/ArrayProfile.h:29, from ../../Source/JavaScriptCore/llint/LLIntOffsetsExtractor.cpp:28: ../../Source/WTF/wtf/Poisoned.h: In substitution of ‘template<uintptr_t& key, class T> using Poisoned = WTF::PoisonedImpl<const long unsigned int&, ((const long unsigned int&)key), T> [with uintptr_t& key = JSC::g_classInfoPoison; T = const JSC::ClassInfo*]’: ../../Source/JavaScriptCore/runtime/JSCPoisonedPtr.h:37:74: required from here ../../Source/WTF/wtf/Poisoned.h:122:56: error: ‘(const long unsigned int&)JSC::g_classInfoPoison’ is not a valid template argument for type ‘const long unsigned int&’ because it is not an object with linkage using Poisoned = PoisonedImpl<const uintptr_t&, key, T>; ^ Committed r225629: <https://trac.webkit.org/changeset/225629> Reopening Comment on attachment 328677 [details] patch for landing + 32-bit windows build fix. View in context: https://bugs.webkit.org/attachment.cgi?id=328677&action=review Here's a GCC 7 version without the bug: https://godbolt.org/g/zVx5fU extern "C" const unsigned bar; template<typename KeyType, KeyType key> struct FooImpl { unsigned boo(unsigned v) { return v ^ key; } }; template<const unsigned& key> using Foo = FooImpl<const unsigned&, key>; Foo<bar> foo; Same thing with the bug: https://godbolt.org/g/a9UYxD extern "C" unsigned bar; template<typename KeyType, KeyType key> struct FooImpl { unsigned boo(unsigned v) { return v ^ key; } }; template<unsigned& key> using Foo = FooImpl<const unsigned&, key>; Foo<bar> foo; > Source/JavaScriptCore/runtime/JSCPoisonedPtr.h:33 > +extern "C" JS_EXPORTDATA uintptr_t g_masmPoison; And const here. > Source/WTF/wtf/Poisoned.h:121 > +template<uintptr_t& key, typename T> key needs to be const here. Or you can drop the const entirely. It'll generate the same code: mov 0x0(%rip),%eax # 0 is a relocation to <bar> xor %edi,%eax Created attachment 328701 [details]
patch for re-landing.
Attachment 328701 [details] did not pass style-queue:
ERROR: Source/JavaScriptCore/runtime/JSCPoisonedPtr.cpp:31: g_classInfoPoison is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/runtime/JSCPoisonedPtr.cpp:32: g_masmPoison is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4]
Total errors found: 2 in 27 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 328701 [details]
patch for re-landing.
r=me
Re-landed in r225632: <http://trac.webkit.org/r225632>. It works, thanks JF! |