148611 – JSC property attributes should fit in a byte

Bug 148611 - JSC property attributes should fit in a byte

Summary: JSC property attributes should fit in a byte

Status:	RESOLVED FIXED

Alias:	None

Product:	WebKit
Classification:	Unclassified
Component:	JavaScriptCore (show other bugs)
Version:	Other
Hardware:	All All

Importance:	P2 Normal
Assignee:	Filip Pizlo

URL:
Keywords:

Depends on:
Blocks:	148610
	Show dependency tree / graph

Reported:	2015-08-29 13:14 PDT by Filip Pizlo
Modified:	2015-08-30 15:34 PDT (History)
CC List:	12 users (show)

See Also:

Attachments
the patch (5.04 KB, patch) 2015-08-29 13:21 PDT, Filip Pizlo	fpizlo: review-	Details \| Formatted Diff \| Diff
a different approach (25.71 KB, patch) 2015-08-29 21:02 PDT, Filip Pizlo	no flags	Details \| Formatted Diff \| Diff
the patch (31.09 KB, patch) 2015-08-29 21:59 PDT, Filip Pizlo	no flags	Details \| Formatted Diff \| Diff
the patch (31.17 KB, patch) 2015-08-29 22:38 PDT, Filip Pizlo	sam: review+	Details \| Formatted Diff \| Diff
Show Obsolete (3) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.

Description Filip Pizlo 2015-08-29 13:14:32 PDT

Patch forthcoming.

Comment 1 Filip Pizlo 2015-08-29 13:21:46 PDT

Created attachment 260226 [details]
the patch

Comment 2 Filip Pizlo 2015-08-29 13:38:04 PDT

Comment on attachment 260226 [details]
the patch

Nope, this doesn't work.

Comment 3 Filip Pizlo 2015-08-29 21:02:08 PDT

Created attachment 260232 [details]
a different approach

Not yet ready for review though.  Just seeing if EWS is happy.

Comment 4 WebKit Commit Bot 2015-08-29 21:03:55 PDT

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


ERROR: Source/JavaScriptCore/runtime/PropertySlot.h:43:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/runtime/PropertySlot.h:47:  One space before end of line comments  [whitespace/comments] [5]
Total errors found: 2 in 7 files


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

Comment 5 Filip Pizlo 2015-08-29 21:59:18 PDT

Created attachment 260236 [details]
the patch

Comment 6 WebKit Commit Bot 2015-08-29 22:01:29 PDT

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


ERROR: Source/JavaScriptCore/runtime/PropertySlot.h:43:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/runtime/PropertySlot.h:47:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/runtime/PropertySlot.h:48:  One space before end of line comments  [whitespace/comments] [5]
Total errors found: 3 in 9 files


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

Comment 7 Filip Pizlo 2015-08-29 22:38:23 PDT

Created attachment 260237 [details]
the patch

Comment 8 WebKit Commit Bot 2015-08-29 22:40:47 PDT

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


ERROR: Source/JavaScriptCore/runtime/PropertySlot.h:43:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/runtime/PropertySlot.h:47:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/runtime/PropertySlot.h:48:  One space before end of line comments  [whitespace/comments] [5]
Total errors found: 3 in 9 files


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

Comment 9 Sam Weinig 2015-08-30 10:26:54 PDT

Is there any reason to keep the static hashtable specific attributes in the Attribute enum anymore? Should we instead have HashTableValue have two fields, m_attributes (of type Attributes) which contains the JSObject attributes and a new m_staticAttributes (awful name) which has the static hashtable specific values?

Comment 10 Filip Pizlo 2015-08-30 10:36:30 PDT

(In reply to comment #9)
> Is there any reason to keep the static hashtable specific attributes in the
> Attribute enum anymore? Should we instead have HashTableValue have two
> fields, m_attributes (of type Attributes) which contains the JSObject
> attributes and a new m_staticAttributes (awful name) which has the static
> hashtable specific values?

I think that might be good follow-up change. The benefit of my approach is that it is a much smaller change, since I don't have to touch the code generators that use the static hash table attributed.

Comment 11 Filip Pizlo 2015-08-30 15:34:34 PDT

Landed in http://trac.webkit.org/changeset/189160