| Summary: | Generate toFooElementData() using ELEMENT_DATA_TYPE_CASTS | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Gyuyoung Kim <gyuyoung.kim> | ||||
| Component: | WebKit Misc. | Assignee: | Gyuyoung Kim <gyuyoung.kim> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Normal | CC: | cmarcelo, commit-queue, darin, d-r, esprehn+autocc, fmalita, kangil.han, pdr, schenney, sergio | ||||
| Priority: | P2 | ||||||
| Version: | 528+ (Nightly build) | ||||||
| Hardware: | Unspecified | ||||||
| OS: | Unspecified | ||||||
| Bug Depends on: | |||||||
| Bug Blocks: | 136773 | ||||||
| Attachments: |
|
||||||
|
Description
Gyuyoung Kim
2014-09-12 00:51:43 PDT
Created attachment 238019 [details]
Patch
CC'ing Darin Comment on attachment 238019 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=238019&action=review > Source/WebCore/dom/ElementData.h:186 > + TYPE_CASTS_BASE(ToValueTypeName, ElementData, elementData, pointerPredicate, referencePredicate) We need to fix TYPE_CASTS_BASE. It’s kind of dumb that we need one expression for -> and one for . in every single invocation. We can use *(x) in the macro to dereference the pointer instead. We should go make that change globally to make every single use of the macro simpler. (In reply to comment #3) > (From update of attachment 238019 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=238019&action=review > > > Source/WebCore/dom/ElementData.h:186 > > + TYPE_CASTS_BASE(ToValueTypeName, ElementData, elementData, pointerPredicate, referencePredicate) > > We need to fix TYPE_CASTS_BASE. It’s kind of dumb that we need one expression for -> and one for . in every single invocation. We can use *(x) in the macro to dereference the pointer instead. We should go make that change globally to make every single use of the macro simpler. I agree your suggestion. It looks it would be nicer to file a new bug to handle this suggestion. I'm going to file a bug for this. (In reply to comment #4) > (In reply to comment #3) > > (From update of attachment 238019 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=238019&action=review > > > > > Source/WebCore/dom/ElementData.h:186 > > > + TYPE_CASTS_BASE(ToValueTypeName, ElementData, elementData, pointerPredicate, referencePredicate) > > > > We need to fix TYPE_CASTS_BASE. It’s kind of dumb that we need one expression for -> and one for . in every single invocation. We can use *(x) in the macro to dereference the pointer instead. We should go make that change globally to make every single use of the macro simpler. > > I agree your suggestion. It looks it would be nicer to file a new bug to handle this suggestion. I'm going to file a bug for this. I file a Bug 136809. I'm gonna upload a patch for this. Comment on attachment 238019 [details] Patch Clearing flags on attachment: 238019 Committed r173610: <http://trac.webkit.org/changeset/173610> All reviewed patches have been landed. Closing bug. (In reply to comment #5) > I file a Bug 136809. I'm gonna upload a patch for this. Bug 136809 is about .get(); doesn’t sound like the same thing to me. Maybe you filed a different bug about the -> vs. dot thing? (In reply to comment #8) > (In reply to comment #5) > > I file a Bug 136809. I'm gonna upload a patch for this. > > Bug 136809 is about .get(); doesn’t sound like the same thing to me. Maybe you filed a different bug about the -> vs. dot thing? Darin, now I understand that you suggested to dereference a pointer in TYPE_CAST_BASE using *(x) use, and then we can adjust it to WebKit globally, right ? If so, Bug 136809 will handle it. If bug title is a little strange, I will change the title when I prepare a patch for the bug. Great, looking forward to seeing it. (In reply to comment #10) > Great, looking forward to seeing it. Now I have very urgent local work, So, I will be able to prepare the patch after going back to my home. :) Please wait for my patch ! Absolutely. I will wait patiently. I may not have a chance to look at it for a while since I have more patch reviewing time on weekends. |