Closed Bug 1459367 Opened Last year Closed Last year

Convert NS_STYLE_IMAGELAYER_ATTACHMENT_* to enum class

Categories

(Core :: CSS Parsing and Computation, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
mozilla62
Tracking Status
firefox62 --- fixed

People

(Reporter: erahm, Assigned: KrisWright, Mentored)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 2 obsolete files)

We would like to convert the |NS_STYLE_IMAGELAYER_ATTACHMENT_*| #defines [1] to an enum class.

Other conversions in nsStyleConsts.h [2, bug 1313565] can be used as an example. The basic steps are:
  #1 - declare an `enum class StyleImageLayerAttachment`
  #2 - convert the #defines to enum values with UpperCamel naming, ie NS_STYLE_IMAGELAYER_ATTACHMENT_SCROL => Scroll
  #3 - replace member variables and params that store NS_STYLE_IMAGELAYER_ATTACHMENT_* values to use the new type, ie `uint8_t mAttachment` => `StyleImageLayerAttachment mAttachment`
  #4 - replace uses of the #defines with the new enum naming, ie NS_STYLE_IMAGELAYER_ATTACHMENT_SCROLL => StyleImageLayerAttachment::Scroll

[1] https://searchfox.org/mozilla-central/rev/53afcfdbabed96883126d0ebbcac499b358e32f2/layout/style/nsStyleConsts.h#271-274
[2] https://searchfox.org/mozilla-central/rev/53afcfdbabed96883126d0ebbcac499b358e32f2/layout/style/nsStyleConsts.h#226-232
Mentor: erahm
Priority: -- → P3
Will not build - new enum does not generate struct in structs.rs
Assignee: nobody → kwright
Status: NEW → ASSIGNED
Comment on attachment 8974187 [details] [diff] [review]
Convert NS_STYLE_IMAGELAYER_ATTACHMENT_* to enum class

Review of attachment 8974187 [details] [diff] [review]:
-----------------------------------------------------------------

This is what we tried. We are still getting the same errors. Could you take a look at this and maybe try it locally?

> 0:36.23 error[E0433]: failed to resolve. Could not find `StyleImageLayerAttachment` in `structs`
> 0:36.23      --> /Users/kristen/src/mozilla-unified/obj-x86_64-apple-darwin17.5.0/toolkit/library/x86_64-apple-darwin/release/build/style-b3afc1a052d9c14f/out/gecko_properties.rs:15505:34
> 0:36.23       |
> 0:36.23 15505 |                         structs::StyleImageLayerAttachment::Scroll ,
> 0:36.23       |                                  ^^^^^^^^^^^^^^^^^^^^^^^^^ Could not find `StyleImageLayerAttachment` in `structs`
> 0:36.23 error[E0433]: failed to resolve. Could not find `StyleImageLayerAttachment` in `structs`
> 0:36.23      --> /Users/kristen/src/mozilla-unified/obj-x86_64-apple-darwin17.5.0/toolkit/library/x86_64-apple-darwin/release/build/style-b3afc1a052d9c14f/out/gecko_properties.rs:15507:34
> 0:36.23       |
> 0:36.23 15507 |                         structs::StyleImageLayerAttachment::Fixed ,
> 0:36.23       |                                  ^^^^^^^^^^^^^^^^^^^^^^^^^ Could not find `StyleImageLayerAttachment` in `structs`
> 0:36.23 error[E0433]: failed to resolve. Could not find `StyleImageLayerAttachment` in `structs`
> 0:36.23      --> /Users/kristen/src/mozilla-unified/obj-x86_64-apple-darwin17.5.0/toolkit/library/x86_64-apple-darwin/release/build/style-b3afc1a052d9c14f/out/gecko_properties.rs:15509:34
> 0:36.23       |
> 0:36.24 15509 |                         structs::StyleImageLayerAttachment::Local ,
> 0:36.24       |                                  ^^^^^^^^^^^^^^^^^^^^^^^^^ Could not find `StyleImageLayerAttachment` in `structs`
> 0:36.24 error[E0433]: failed to resolve. Could not find `StyleImageLayerAttachment` in `structs`
> 0:36.24      --> /Users/kristen/src/mozilla-unified/obj-x86_64-apple-darwin17.5.0/toolkit/library/x86_64-apple-darwin/release/build/style-b3afc1a052d9c14f/out/gecko_properties.rs:15524:34
> 0:36.24       |
> 0:36.24 15524 |                         structs::StyleImageLayerAttachment::Scroll
> 0:36.24       |                                  ^^^^^^^^^^^^^^^^^^^^^^^^^ Could not find `StyleImageLayerAttachment` in `structs`
> 0:36.24 error[E0433]: failed to resolve. Could not find `StyleImageLayerAttachment` in `structs`
> 0:36.24      --> /Users/kristen/src/mozilla-unified/obj-x86_64-apple-darwin17.5.0/toolkit/library/x86_64-apple-darwin/release/build/style-b3afc1a052d9c14f/out/gecko_properties.rs:15526:34
> 0:36.24       |
> 0:36.24 15526 |                         structs::StyleImageLayerAttachment::Fixed
> 0:36.24       |                                  ^^^^^^^^^^^^^^^^^^^^^^^^^ Could not find `StyleImageLayerAttachment` in `structs`
> 0:36.24 error[E0433]: failed to resolve. Could not find `StyleImageLayerAttachment` in `structs`
> 0:36.24      --> /Users/kristen/src/mozilla-unified/obj-x86_64-apple-darwin17.5.0/toolkit/library/x86_64-apple-darwin/release/build/style-b3afc1a052d9c14f/out/gecko_properties.rs:15528:34
> 0:36.24       |
> 0:36.24 15528 |                         structs::StyleImageLayerAttachment::Local
> 0:36.24       |                                  ^^^^^^^^^^^^^^^^^^^^^^^^^ Could not find `StyleImageLayerAttachment` in `structs`
Attachment #8974187 - Flags: feedback?(manishearth)
Comment on attachment 8974187 [details] [diff] [review]
Convert NS_STYLE_IMAGELAYER_ATTACHMENT_* to enum class

Review of attachment 8974187 [details] [diff] [review]:
-----------------------------------------------------------------

You probably just need to add it to the list at https://searchfox.org/mozilla-central/rev/5ff2d7683078c96e4b11b8a13674daded935aa44/layout/style/ServoBindings.toml#151
Yeah, things need to be listed there too.
Will not build - type errors
Comment on attachment 8974462 [details] [diff] [review]
Convert NS_STYLE_IMAGELAYER_ATTACHMENT_* to enum class

Review of attachment 8974462 [details] [diff] [review]:
-----------------------------------------------------------------

Thank you so much for the help thus far. We were able to update the servo bindings but it is now trying to read as a u8, even though its entry in background.mako.rs has been modified from constant to enum. Do you have any pointers as to why we are getting this error or how to fix it?

> 1:03.69 error[E0308]: match arms have incompatible types
> 1:03.69      --> /Users/kristen/src/mozilla-unified/obj-x86_64-apple-darwin17.5.0/toolkit/library/x86_64-apple-darwin/release/build/style-b3afc1a052d9c14f/out/gecko_properties.rs:15503:17
> 1:03.69       |
> 1:03.69 15503 | /                 match servo {
> 1:03.69 15504 | |                     Keyword::Scroll =>
> 1:03.69 15505 | |                         structs::StyleImageLayerAttachment::Scroll ,
> 1:03.69       | |                         ------------------------------------------ match arm with an incompatible type
> 1:03.69 15506 | |                     Keyword::Fixed =>
> 1:03.69 ...     |
> 1:03.69 15509 | |                         structs::StyleImageLayerAttachment::Local ,
> 1:03.69 15510 | |                 }
> 1:03.69       | |_________________^ expected u8, found enum `gecko_bindings::structs::root::mozilla::StyleImageLayerAttachment`
> 1:03.69       |
> 1:03.69       = note: expected type `u8`
> 1:03.69                  found type `gecko_bindings::structs::root::mozilla::StyleImageLayerAttachment`
> 1:03.69 error[E0308]: mismatched types
> 1:03.69      --> /Users/kristen/src/mozilla-unified/obj-x86_64-apple-darwin17.5.0/toolkit/library/x86_64-apple-darwin/release/build/style-b3afc1a052d9c14f/out/gecko_properties.rs:15524:25
> 1:03.69       |
> 1:03.69 15524 |                         structs::StyleImageLayerAttachment::Scroll
> 1:03.69       |                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected u8, found enum `gecko_bindings::structs::root::mozilla::StyleImageLayerAttachment`
> 1:03.70       |
> 1:03.70       = note: expected type `u8`
> 1:03.70                  found type `gecko_bindings::structs::root::mozilla::StyleImageLayerAttachment`
> 1:03.70 error[E0308]: mismatched types
> 1:03.70      --> /Users/kristen/src/mozilla-unified/obj-x86_64-apple-darwin17.5.0/toolkit/library/x86_64-apple-darwin/release/build/style-b3afc1a052d9c14f/out/gecko_properties.rs:15526:25
> 1:03.70       |
> 1:03.70 15526 |                         structs::StyleImageLayerAttachment::Fixed
> 1:03.70       |                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected u8, found enum `gecko_bindings::structs::root::mozilla::StyleImageLayerAttachment`
> 1:03.70       |
> 1:03.70       = note: expected type `u8`
> 1:03.70                  found type `gecko_bindings::structs::root::mozilla::StyleImageLayerAttachment`
> 1:03.70 error[E0308]: mismatched types
> 1:03.70      --> /Users/kristen/src/mozilla-unified/obj-x86_64-apple-darwin17.5.0/toolkit/library/x86_64-apple-darwin/release/build/style-b3afc1a052d9c14f/out/gecko_properties.rs:15528:25
> 1:03.70       |
> 1:03.70 15528 |                         structs::StyleImageLayerAttachment::Local
> 1:03.70       |                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected u8, found enum `gecko_bindings::structs::root::mozilla::StyleImageLayerAttachment`
> 1:03.70       |
> 1:03.70       = note: expected type `u8`
> 1:03.70                  found type `gecko_bindings::structs::root::mozilla::StyleImageLayerAttachment`
Attachment #8974462 - Flags: feedback?(manishearth)
Comment on attachment 8974462 [details] [diff] [review]
Convert NS_STYLE_IMAGELAYER_ATTACHMENT_* to enum class

I can probably help out from here. It looks like `mAttachment` wasn't updated to the new enum class type.
Attachment #8974462 - Flags: feedback?(manishearth)
Attachment #8974187 - Flags: feedback?(manishearth)
Attachment #8974187 - Attachment is obsolete: true
Attachment #8974462 - Attachment is obsolete: true
Converted NS_STYLE_IMAGELAYER_ATTATCHMENT_* vals to enum class, StyleImageLayerAttachment.
Attachment #8974530 - Flags: review?(manishearth)
Attachment #8974530 - Flags: review?(manishearth) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/00dd151d33d3f6d7f21c7dd450ca3b81ec061249
Bug 1459367 - Convert NS_STYLE_IMAGELAYER_ATTACHMENT_* to enum class. r=manishearth
https://hg.mozilla.org/mozilla-central/rev/00dd151d33d3
Status: ASSIGNED → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
You need to log in before you can comment on or make changes to this bug.