Bug 140501 - Generate StylePropertyShorthand.* from CSSPropertyNames.in
Summary: Generate StylePropertyShorthand.* from CSSPropertyNames.in
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords:
Depends on: 140568
Blocks: 140556 140599
  Show dependency treegraph
 
Reported: 2015-01-15 10:41 PST by Chris Dumez
Modified: 2015-01-18 16:00 PST (History)
7 users (show)

See Also:


Attachments
Patch (64.02 KB, patch)
2015-01-15 15:37 PST, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (64.87 KB, patch)
2015-01-15 16:38 PST, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (64.90 KB, patch)
2015-01-15 16:53 PST, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (68.25 KB, patch)
2015-01-16 10:48 PST, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 2015-01-15 10:41:16 PST
Most of StylePropertyShorthand.* is boilerplate code that could easily be generated from CSSProperties.in file if we included in it the longhands for each shorthand property.
Comment 1 Chris Dumez 2015-01-15 15:37:27 PST
Created attachment 244719 [details]
Patch
Comment 2 Chris Dumez 2015-01-15 16:38:12 PST
Created attachment 244724 [details]
Patch
Comment 3 Chris Dumez 2015-01-15 16:53:18 PST
Created attachment 244729 [details]
Patch
Comment 4 Chris Dumez 2015-01-15 17:27:44 PST
Comment on attachment 244729 [details]
Patch

Patch is ready for review.
Comment 5 Darin Adler 2015-01-16 09:32:31 PST
Comment on attachment 244729 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=244729&action=review

> Source/WebCore/css/makeprop.pl:956
> +  print SHORTHANDS_CPP "    return StylePropertyShorthand(CSSProperty" . $nameToId{$name} . ", " . $lowercaseId . "Properties, WTF_ARRAY_LENGTH(" . $lowercaseId . "Properties));\n";

Seems like we could avoid this WTF_ARRAY_LENGTH if we made the constructor a template that takes an array and deduces its length, like StringBuilder::appendLiteral, for example. Anything that reduces the amount of generated code and puts more of the code in normal C++ header files seems like a win to me. To state the obvious, we want the code generator to generate things that need to be repeated but make the actual code generated as minimal as we can.
Comment 6 Chris Dumez 2015-01-16 10:48:23 PST
Created attachment 244773 [details]
Patch
Comment 7 Chris Dumez 2015-01-16 11:28:20 PST
(In reply to comment #5)
> Comment on attachment 244729 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=244729&action=review
> 
> > Source/WebCore/css/makeprop.pl:956
> > +  print SHORTHANDS_CPP "    return StylePropertyShorthand(CSSProperty" . $nameToId{$name} . ", " . $lowercaseId . "Properties, WTF_ARRAY_LENGTH(" . $lowercaseId . "Properties));\n";
> 
> Seems like we could avoid this WTF_ARRAY_LENGTH if we made the constructor a
> template that takes an array and deduces its length, like
> StringBuilder::appendLiteral, for example. Anything that reduces the amount
> of generated code and puts more of the code in normal C++ header files seems
> like a win to me. To state the obvious, we want the code generator to
> generate things that need to be repeated but make the actual code generated
> as minimal as we can.

I made the suggested change before landing.
Comment 8 WebKit Commit Bot 2015-01-16 11:39:01 PST
Comment on attachment 244773 [details]
Patch

Clearing flags on attachment: 244773

Committed r178586: <http://trac.webkit.org/changeset/178586>
Comment 9 WebKit Commit Bot 2015-01-16 11:39:07 PST
All reviewed patches have been landed.  Closing bug.