Bug 183573 – Make a NativeFunction into a class to support pointer profiling.

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Mark Lam]

We'll also introduce RawNativeFunction as the type of the raw pointer, and TaggedNativeFunction as the tagged version.

[Radar WebKit Bug Importer]

<rdar://problem/38384697>

[Mark Lam]

Created attachment 335626 [details]
proposed patch.

[EWS Watchlist]

Attachment 335626 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/runtime/NativeFunction.h:40:  Should be indented on a separate line, with the colon or comma first on that line.  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/runtime/NativeFunction.h:41:  Should be indented on a separate line, with the colon or comma first on that line.  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/runtime/NativeFunction.h:42:  Should be indented on a separate line, with the colon or comma first on that line.  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/runtime/NativeFunction.h:69:  Should be indented on a separate line, with the colon or comma first on that line.  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/runtime/NativeFunction.h:70:  Should be indented on a separate line, with the colon or comma first on that line.  [whitespace/indent] [4]
Total errors found: 5 in 67 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Mark Lam]

Created attachment 335628 [details]
proposed patch.

[EWS Watchlist]

Attachment 335628 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/runtime/NativeFunction.h:40:  Should be indented on a separate line, with the colon or comma first on that line.  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/runtime/NativeFunction.h:41:  Should be indented on a separate line, with the colon or comma first on that line.  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/runtime/NativeFunction.h:42:  Should be indented on a separate line, with the colon or comma first on that line.  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/runtime/NativeFunction.h:69:  Should be indented on a separate line, with the colon or comma first on that line.  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/runtime/NativeFunction.h:70:  Should be indented on a separate line, with the colon or comma first on that line.  [whitespace/indent] [4]
Total errors found: 5 in 67 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Mark Lam]

Created attachment 335632 [details]
proposed patch.

[EWS Watchlist]

Attachment 335632 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/runtime/NativeFunction.h:40:  Should be indented on a separate line, with the colon or comma first on that line.  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/runtime/NativeFunction.h:41:  Should be indented on a separate line, with the colon or comma first on that line.  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/runtime/NativeFunction.h:42:  Should be indented on a separate line, with the colon or comma first on that line.  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/runtime/NativeFunction.h:69:  Should be indented on a separate line, with the colon or comma first on that line.  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/runtime/NativeFunction.h:70:  Should be indented on a separate line, with the colon or comma first on that line.  [whitespace/indent] [4]
Total errors found: 5 in 68 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Filip Pizlo]

Comment on attachment 335632 [details]
proposed patch.

Nice!

[Mark Lam]

Thanks for the review.  Landed in r229547: <http://trac.webkit.org/r229547>.

[Ryan Haddad]

This change broke the Windows build:

https://build.webkit.org/builders/Apple%20Win%20Release%20%28Build%29/builds/8246

C:\cygwin\home\buildbot\slave\win-release\build\Source\JavaScriptCore\jit\JITThunks.h(103): error C2220: warning treated as error - no 'object' file generated [C:\cygwin\home\buildbot\slave\win-release\build\WebKitBuild\Release\Source\JavaScriptCore\LLIntOffsetsExtractor.vcxproj]
C:\cygwin\home\buildbot\slave\win-release\build\Source\JavaScriptCore\jit\JITThunks.h(103): warning C4245: 'argument': conversion from 'int' to 'uintptr_t', signed/unsigned mismatch [C:\cygwin\home\buildbot\slave\win-release\build\WebKitBuild\Release\Source\JavaScriptCore\LLIntOffsetsExtractor.vcxproj]
C:\cygwin\home\buildbot\slave\win-release\build\Source\JavaScriptCore\jit\JITThunks.h(104): warning C4245: 'argument': conversion from 'int' to 'uintptr_t', signed/unsigned mismatch [C:\cygwin\home\buildbot\slave\win-release\build\WebKitBuild\Release\Source\JavaScriptCore\LLIntOffsetsExtractor.vcxproj]

[Mark Lam]

(In reply to Ryan Haddad from comment #10)
> This change broke the Windows build:
> ...

Speculative fix landed in r229557: <http://trac.webkit.org/r229557>.

[Ross Kirsling]

WinCairo errors after r229557:
(https://build.webkit.org/builders/WinCairo%2064-Bit%20Release/builds/11559)

C:\WebKit-EWS\WebKit\Source\JavaScriptCore\jit\JITThunks.h(103): error C2440: '<function-style-cast>': cannot convert from 'int' to 'JSC::TaggedNativeFunction'
C:\WebKit-EWS\WebKit\Source\JavaScriptCore\jit\JITThunks.h(103): note: No constructor could take the source type, or constructor overload resolution was ambiguous
C:\WebKit-EWS\WebKit\Source\JavaScriptCore\jit\JITThunks.h(104): error C2440: '<function-style-cast>': cannot convert from 'int' to 'JSC::TaggedNativeFunction'
C:\WebKit-EWS\WebKit\Source\JavaScriptCore\jit\JITThunks.h(104): note: No constructor could take the source type, or constructor overload resolution was ambiguous

[Mark Lam]

(In reply to Ross Kirsling from comment #12)
> WinCairo errors after r229557:
> (https://build.webkit.org/builders/WinCairo%2064-Bit%20Release/builds/11559)
> 
> C:\WebKit-EWS\WebKit\Source\JavaScriptCore\jit\JITThunks.h(103): error
> C2440: '<function-style-cast>': cannot convert from 'int' to
> 'JSC::TaggedNativeFunction'
> C:\WebKit-EWS\WebKit\Source\JavaScriptCore\jit\JITThunks.h(103): note: No
> constructor could take the source type, or constructor overload resolution
> was ambiguous
> C:\WebKit-EWS\WebKit\Source\JavaScriptCore\jit\JITThunks.h(104): error
> C2440: '<function-style-cast>': cannot convert from 'int' to
> 'JSC::TaggedNativeFunction'
> C:\WebKit-EWS\WebKit\Source\JavaScriptCore\jit\JITThunks.h(104): note: No
> constructor could take the source type, or constructor overload resolution
> was ambiguous

AppleWin bots shows that this issue has already been fixed (see build #8250).  The fix landed in r22955.  Are you seeing a build failure after that revision?

[Mark Lam]

(In reply to Mark Lam from comment #13)
> AppleWin bots shows that this issue has already been fixed (see build
> #8250).  The fix landed in r22955.  Are you seeing a build failure after
> that revision?

Forgot the build bot url: https://build.webkit.org/builders/Apple%20Win%20Release%20(Build)?numbuilds=50

[Ross Kirsling]

AppleWin went from red to green but WinCairo went from green to red. :)
https://build.webkit.org/builders/WinCairo%2064-Bit%20Release?numbuilds=50

[Mark Lam]

(In reply to Ross Kirsling from comment #15)
> AppleWin went from red to green but WinCairo went from green to red. :)
> https://build.webkit.org/builders/WinCairo%2064-Bit%20Release?numbuilds=50

Speculative build fix for WinCairo landed in r229574: <http://trac.webkit.org/r229574>.

[Ross Kirsling]

(In reply to Mark Lam from comment #16)
> (In reply to Ross Kirsling from comment #15)
> > AppleWin went from red to green but WinCairo went from green to red. :)
> > https://build.webkit.org/builders/WinCairo%2064-Bit%20Release?numbuilds=50
> 
> Speculative build fix for WinCairo landed in r229574:
> <http://trac.webkit.org/r229574>.

Thanks much!