Bug 216322

Summary: [WebIDL] Stop automatically applying the ImplementedBy extended attribute to all partial interfaces/dictionaries
Product: WebKit Reporter: Sam Weinig <sam>
Component: New BugsAssignee: Sam Weinig <sam>
Status: RESOLVED FIXED    
Severity: Normal CC: alecflett, beidson, benjamin, calvaris, cdumez, changseok, darin, dino, eric.carlson, esprehn+autocc, ews-watchlist, fmalita, glenn, gyuyoung.kim, hta, jer.noble, jiewen_tan, joepeck, jsbell, kangil.han, kondapallykalyan, macpherson, menard, mmaxfield, pdr, philipj, sabouhallawa, schenney, sergio, tommyw, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Sam Weinig
Reported 2020-09-09 13:19:00 PDT
[WebIDL] Stop automatically applying the ImplementedBy extended attribute to all partial interfaces/dictionaries
Attachments
Patch (33.72 KB, patch)
2020-09-09 13:42 PDT, Sam Weinig 		no flags
DetailsFormatted DiffDiff
Patch (33.72 KB, patch)
2020-09-09 14:14 PDT, Sam Weinig 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Sam Weinig
Comment 1 2020-09-09 13:42:15 PDT
Created attachment 408363 [details] Patch
Darin Adler
Comment 2 2020-09-09 13:57:11 PDT
Comment on attachment 408363 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=408363&action=review > Source/WebCore/ChangeLog:13 > + behavior mush opt in, using the extended attribute "ImplementedBy", which is what the code typo: "must" > Source/WebCore/ChangeLog:25 > + Remove special casing of partial interfaces / dictionaries automatically getting "ImplementedBy" > + applied to all members. Maintain the behavior that "ImplementedBy" should not be used when > + an attribute has been marked as "Reflect". When we say "should not be used" we mean "should be silently ignored"? Is that right? That seems to be new. It’s one thing to not implicitly add ImplementedBy when Reflect is specified. Not sure it’s right to ignore it rather than emitting an error message.
Sam Weinig
Comment 3 2020-09-09 14:11:28 PDT
(In reply to Darin Adler from comment #2) > Comment on attachment 408363 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=408363&action=review > > > Source/WebCore/ChangeLog:13 > > + behavior mush opt in, using the extended attribute "ImplementedBy", which is what the code > > typo: "must" > > > Source/WebCore/ChangeLog:25 > > + Remove special casing of partial interfaces / dictionaries automatically getting "ImplementedBy" > > + applied to all members. Maintain the behavior that "ImplementedBy" should not be used when > > + an attribute has been marked as "Reflect". > > When we say "should not be used" we mean "should be silently ignored"? Is > that right? Yeah, that is more clear. > That seems to be new. It’s one thing to not implicitly add > ImplementedBy when Reflect is specified. Not sure it’s right to ignore it > rather than emitting an error message. The old behavior was that we would skip adding ImplementedBy (and therefore using the MyClass::staticFunction(...) syntax) if an attribute in partial interface had [Reflect]: - if ($interface->isPartial && !$interface->isMixin) { - $attribute->extendedAttributes->{"ImplementedBy"} = $interfaceName if !$attribute->extendedAttributes->{Reflect}; - } I don't think this ends up being that confusing as [Reflect] really means "there is not attribute getter/setter for this, instead call the right hasAttributeWithoutSynchronization (or whatever the appropriate one is for the type) and pass in the appropriate qualified name). If we remove this special case, it would mean instead of specifying ImplementedBy on the partial interface itself, we would have to do it on each member. I think I am ok with keeping this little bit of weirdness.
Sam Weinig
Comment 4 2020-09-09 14:14:17 PDT
Created attachment 408367 [details] Patch
EWS
Comment 5 2020-09-09 15:11:36 PDT
Committed r266800: <https://trac.webkit.org/changeset/266800> All reviewed patches have been landed. Closing bug and clearing flags on attachment 408367 [details].
Radar WebKit Bug Importer
Comment 6 2020-09-09 15:12:17 PDT
<rdar://problem/68598852>
Note You need to log in before you can comment on or make changes to this bug.