Replace [ConstructorRaisesException] with [RaisesException]

• Component: Bindings
• Status: RESOLVED WONTFIX

[Chris Dumez]

[RaisesException] extended attribute was added in Bug 117350 for Web IDL operations. [ConstructorRaisesException] was the pre-existing equivalent for interface constructors. It does not make sense to have 2 Web IDL extended attributes to do the same thing so we should consider replacing [ConstructorRaisesException] with [RaisesException] as blink did.

[Chris Dumez]

Created attachment 204148 [details]
Patch

[Kentaro Hara]

Comment on attachment 204148 [details]
Patch

ok

[WebKit Commit Bot]

Comment on attachment 204148 [details]
Patch

Clearing flags on attachment: 204148

Committed r151378: <http://trac.webkit.org/changeset/151378>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Darin Adler]

This change seems strange. Now we label an entire interface “RaisesException” and to me it’s not obvious what that means.

[Chris Dumez]

(In reply to comment #5)
> This change seems strange. Now we label an entire interface “RaisesException” and to me it’s not obvious what that means.

I see. I was trying to reduce the number of WebKit-specific IDL attributes (and be consistent with Blink IDL). However, I see your point that we do loose some readability in this case (and people would likely have to refer to the WebKit IDL wiki).

It did not disturb me much because a similar thing was already done for:
http://trac.webkit.org/wiki/WebKitIDL#CallWith

If you specify [CallWith] on an interface, it applies only to the constructor, not for all attributes / operations of the interface. The attribute could have been called [CallConstructorWith] for clarity but [CallWith] is used (probably for consistency with operations / attributes). Having a limited number of Webkit-specific IDL attributes also has benefits.

I don't have strong feelings either way (only slight preference was keeping the patch in). Therefore, if you prefer to keep [ConstructorRaisesException], then I don't mind reverting this patch. However, I would then propose that we introduce [CallConstructorWith] to follow the same pattern.

[Darin Adler]

I agree that CallConstructorWith is clearer than CallWith.

I don’t agree that same attribute name used as an interface attribute and an operation attribute with subtly different meanings counts as a smaller number of attributes.

We are optimizing a number, number of distinct attribute names, that is not a helpful metric.

I agree that minimizing the number of attributes is important. Although, as an aside, I think it is more important to minimize the degree to which class names and behavior are hardcoded into the bindings generation scripts than to minimize the number of attributes.

But I believe that reusing the same name for multiple subtly different purposes is a false economy.

[Chris Dumez]

(In reply to comment #7)
> I agree that CallConstructorWith is clearer than CallWith.
> 
> I don’t agree that same attribute name used as an interface attribute and an operation attribute with subtly different meanings counts as a smaller number of attributes.
> 
> We are optimizing a number, number of distinct attribute names, that is not a helpful metric.
> 
> I agree that minimizing the number of attributes is important. Although, as an aside, I think it is more important to minimize the degree to which class names and behavior are hardcoded into the bindings generation scripts than to minimize the number of attributes.
> 
> But I believe that reusing the same name for multiple subtly different purposes is a false economy.

Ok, I'm not disagreeing with you. I will roll this out soon then and likely propose a small patch to introduce CallConstructorWith.

[Kentaro Hara]

> Ok, I'm not disagreeing with you. I will roll this out soon then and likely propose a small patch to introduce CallConstructorWith.

Bikeshedding: [ConstructorCallWith] might be slightly better, since we might want to prefix all constructor related IDL attributes with "Constructor".

Historically, previously [CallWith] was [ConstructorCallWith]. I replaced [ConstructorCallWith] with [CallWith] at some point.

[Chris Dumez]

Reverted r151378 for reason:

Decreased IDL readability a bit

Committed r151487: <http://trac.webkit.org/changeset/151487>

[Chris Dumez]

(In reply to comment #9)
> > Ok, I'm not disagreeing with you. I will roll this out soon then and likely propose a small patch to introduce CallConstructorWith.
> 
> Bikeshedding: [ConstructorCallWith] might be slightly better, since we might want to prefix all constructor related IDL attributes with "Constructor".
> 
> Historically, previously [CallWith] was [ConstructorCallWith]. I replaced [ConstructorCallWith] with [CallWith] at some point.

Makes sense Kentaro. I'll use that instead then, thanks.

[Darin Adler]

Thanks guys. Hope my wording didn’t come across as harsh. I may have prioritized clarity over politeness a bit!

[Chris Dumez]

(In reply to comment #12)
> Thanks guys. Hope my wording didn’t come across as harsh. I may have prioritized clarity over politeness a bit!

Not at all :) Thanks for the feedback.