Bug 206816

Summary: Make getVTablePointer() an inline function to be compliant with WebKit style guidelines.
Product: WebKit Reporter: Mark Lam <mark.lam>
Component: JavaScriptCoreAssignee: Mark Lam <mark.lam>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, cdumez, cmarcelo, commit-queue, darin, dbates, ews-watchlist, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=206804
Attachments:
Description Flags
proposed patch.
darin: review+
patch for landing.
none
patch for landing. none

Mark Lam
Reported 2020-01-26 21:19:01 PST
See https://bugs.webkit.org/show_bug.cgi?id=206804#c7.
Attachments
proposed patch. (1.85 KB, patch)
2020-01-26 21:22 PST, Mark Lam 		darin: review+
DetailsFormatted DiffDiff
patch for landing. (2.01 KB, patch)
2020-01-26 21:48 PST, Mark Lam 		no flags
DetailsFormatted DiffDiff
patch for landing. (2.10 KB, patch)
2020-01-26 22:06 PST, Mark Lam 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Mark Lam
Comment 1 2020-01-26 21:22:37 PST
Created attachment 388823 [details] proposed patch.
Darin Adler
Comment 2 2020-01-26 21:41:23 PST
Comment on attachment 388823 [details] proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=388823&action=review > Source/WTF/wtf/PointerPreparations.h:36 > #if __has_builtin(__builtin_get_vtable_pointer) Why is this after the CPU(ARM64E) check. Wouldn't we want to use the builtin if it exists, regardless of what the CPU is?
Mark Lam
Comment 3 2020-01-26 21:43:19 PST
Comment on attachment 388823 [details] proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=388823&action=review >> Source/WTF/wtf/PointerPreparations.h:36 >> #if __has_builtin(__builtin_get_vtable_pointer) > > Why is this after the CPU(ARM64E) check. Wouldn't we want to use the builtin if it exists, regardless of what the CPU is? Good point. I'll rearrange the #ifs. Thanks.
Mark Lam
Comment 4 2020-01-26 21:48:58 PST
Created attachment 388824 [details] patch for landing.
Darin Adler
Comment 5 2020-01-26 22:00:21 PST
Comment on attachment 388824 [details] patch for landing. View in context: https://bugs.webkit.org/attachment.cgi?id=388824&action=review > Source/WTF/wtf/PointerPreparations.h:34 > #if __has_builtin(__builtin_get_vtable_pointer) Oops, this isn’t good for non-clang compilers, like on Windows.
Darin Adler
Comment 6 2020-01-26 22:01:38 PST
Comment on attachment 388824 [details] patch for landing. View in context: https://bugs.webkit.org/attachment.cgi?id=388824&action=review >> Source/WTF/wtf/PointerPreparations.h:34 >> #if __has_builtin(__builtin_get_vtable_pointer) > > Oops, this isn’t good for non-clang compilers, like on Windows. What Anders did for things like this in Compiler.h is: #if defined(__has_builtin) && __has_builtin(__builtin_get_vtable_pointer) This apparently works on all the compilers.
Mark Lam
Comment 7 2020-01-26 22:06:13 PST
Created attachment 388826 [details] patch for landing.
WebKit Commit Bot
Comment 8 2020-01-27 00:54:53 PST
Comment on attachment 388826 [details] patch for landing. Clearing flags on attachment: 388826 Committed r255139: <https://trac.webkit.org/changeset/255139>
WebKit Commit Bot
Comment 9 2020-01-27 00:54:55 PST
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 10 2020-01-27 00:55:18 PST
<rdar://problem/58911518>
Note You need to log in before you can comment on or make changes to this bug.