Bug 231158

Summary: Make RuleSetBuilder a class
Product: WebKit Reporter: Antti Koivisto <koivisto>
Component: CSSAssignee: Antti Koivisto <koivisto>
Status: RESOLVED FIXED    
Severity: Normal CC: darin, webkit-bug-importer, zalan
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch none

Antti Koivisto
Reported 2021-10-04 01:41:33 PDT
improve encapsulation
Attachments
Patch (36.23 KB, patch)
2021-10-04 01:45 PDT, Antti Koivisto
no flags
Patch (36.08 KB, patch)
2021-10-04 01:51 PDT, Antti Koivisto
no flags
Patch (36.08 KB, patch)
2021-10-04 02:10 PDT, Antti Koivisto
no flags
Patch (36.20 KB, patch)
2021-10-04 06:19 PDT, Antti Koivisto
no flags
Patch (36.07 KB, patch)
2021-10-04 06:53 PDT, Antti Koivisto
no flags
Patch (36.12 KB, patch)
2021-10-04 07:45 PDT, Antti Koivisto
no flags
Patch (36.07 KB, patch)
2021-10-04 07:49 PDT, Antti Koivisto
no flags
Patch (36.07 KB, patch)
2021-10-04 07:52 PDT, Antti Koivisto
no flags
Antti Koivisto
Comment 1 2021-10-04 01:45:02 PDT
Antti Koivisto
Comment 2 2021-10-04 01:51:46 PDT
Antti Koivisto
Comment 3 2021-10-04 02:10:31 PDT
Antti Koivisto
Comment 4 2021-10-04 06:19:56 PDT
Antti Koivisto
Comment 5 2021-10-04 06:53:50 PDT
Antti Koivisto
Comment 6 2021-10-04 07:45:25 PDT
Antti Koivisto
Comment 7 2021-10-04 07:49:18 PDT
Antti Koivisto
Comment 8 2021-10-04 07:52:22 PDT
EWS
Comment 9 2021-10-04 12:34:12 PDT
Committed r283508 (242476@main): <https://commits.webkit.org/242476@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 440062 [details].
Radar WebKit Bug Importer
Comment 10 2021-10-04 12:35:21 PDT
Darin Adler
Comment 11 2021-10-04 13:21:11 PDT
Comment on attachment 440062 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=440062&action=review Noticed one small stylistic thing. > Source/WebCore/style/RuleSetBuilder.h:59 > + Vector<size_t> affectedRulePositions { }; > + RuleFeatureVector ruleFeatures { }; These braces aren’t needed. > Source/WebCore/style/RuleSetBuilder.h:63 > + Vector<DynamicContext> dynamicContextStack { }; > + > + Vector<RuleSet::DynamicMediaQueryRules> dynamicMediaQueryRules { }; These braces aren’t needed. > Source/WebCore/style/RuleSetBuilder.h:75 > + CascadeLayerName m_resolvedCascadeLayerName { }; > + HashMap<CascadeLayerName, RuleSet::CascadeLayerIdentifier> m_cascadeLayerIdentifierMap { }; These braces aren’t needed. > Source/WebCore/style/RuleSetBuilder.h:77 > + Vector<RuleSet::ResolverMutatingRule> m_collectedResolverMutatingRules { }; These braces aren’t needed.
Antti Koivisto
Comment 12 2021-10-04 23:59:13 PDT
Comment on attachment 440062 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=440062&action=review >> Source/WebCore/style/RuleSetBuilder.h:59 >> + struct DynamicContext { >> + Ref<const MediaQuerySet> set; >> + Vector<size_t> affectedRulePositions { }; >> + RuleFeatureVector ruleFeatures { }; > > These braces aren’t needed. Some of the compilers in bots have failed in the past if all fields are not initialized explicitly when the struct is brace-initialized.
Darin Adler
Comment 13 2021-10-05 09:49:39 PDT
Comment on attachment 440062 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=440062&action=review >>> Source/WebCore/style/RuleSetBuilder.h:59 >>> + RuleFeatureVector ruleFeatures { }; >> >> These braces aren’t needed. > > Some of the compilers in bots have failed in the past if all fields are not initialized explicitly when the struct is brace-initialized. Does that really apply here or not?
Antti Koivisto
Comment 14 2021-10-05 11:44:15 PDT
> Does that really apply here or not? Seems so, see https://bugs.webkit.org/show_bug.cgi?id=231236 (class members here don't need them)
Darin Adler
Comment 15 2021-10-05 12:50:12 PDT
(In reply to Antti Koivisto from comment #14) > > Does that really apply here or not? > > Seems so, see https://bugs.webkit.org/show_bug.cgi?id=231236 > > (class members here don't need them) Thanks! I appreciate learning about this.
Note You need to log in before you can comment on or make changes to this bug.