| Summary: | Remove UsePointersEvenForNonNullableObjectArguments keyword | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | youenn fablet <youennf> | ||||||
| Component: | WebCore JavaScript | Assignee: | youenn fablet <youennf> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | cdumez, cgarcia, commit-queue, darin, ddkilzer | ||||||
| Priority: | P2 | ||||||||
| Version: | WebKit Nightly Build | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Bug Depends on: | 156897, 156898, 156899, 156901, 156903, 156904, 156905, 156908, 156909, 156977, 156978, 156979 | ||||||||
| Bug Blocks: | |||||||||
| Attachments: |
|
||||||||
|
Description
youenn fablet
2016-04-21 03:40:14 PDT
Created attachment 276905 [details]
Patch
I don’t agree with this strategy. Instead I suggest we mark arguments as nullable and then remove the nullability over time. Basically, I don’t think we need two different forms of nullable, one that is "nullable on purpose" and another that is "nullable but should be fixed". To me that seems over-engineered. I would also prefer to use regular nullable (?) with a FIXME comment rather than yet another WebKit-specific IDL attribute. I am looking forward to getting rid of [UsePointersEvenForNonNullableObjectArguments] entirely as soon as possible. OK, I'll update the patch to remove this keyword using the FIXME approach. ddkilzer, is it ok to remove support for this keyword in the binding generator? (In reply to comment #5) > OK, I'll update the patch to remove this keyword using the FIXME approach. > > ddkilzer, is it ok to remove support for this keyword in the binding > generator? No, not yet. Someone would need to clean up non-OpenSource ones first :/ Can we do it IDL file by IDL file (like we've been doing so far) rather than in one huge patch please? It is easier to review carefully and make sure the nullable is actually needed and the FIXME comments are accurate. For e.g., ideally I would not want to add a nullable in the IDL if the implementation does a null-check and throws when getting a null pointer. Also FIXME comments need to be added only when the added nullable in the IDL does not match the corresponding spec. (In reply to comment #6) > (In reply to comment #5) > > OK, I'll update the patch to remove this keyword using the FIXME approach. > > > > ddkilzer, is it ok to remove support for this keyword in the binding > > generator? > > No, not yet. Someone would need to clean up non-OpenSource ones first :/ I'll take care of this. Yes, lets keep doing this one file at a time! (In reply to comment #6) > (In reply to comment #5) > > OK, I'll update the patch to remove this keyword using the FIXME approach. > > > > ddkilzer, is it ok to remove support for this keyword in the binding > > generator? > > No, not yet. Someone would need to clean up non-OpenSource ones first :/ Ok, it should now be safe once we get rid of all the uses in trunk. Created attachment 277675 [details]
Patch
Comment on attachment 277675 [details] Patch Clearing flags on attachment: 277675 Committed r200236: <http://trac.webkit.org/changeset/200236> All reviewed patches have been landed. Closing bug. |