Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make font-feature-settings a subprop of font #16827

Merged

Conversation

upsuper
Copy link
Contributor

@upsuper upsuper commented May 12, 2017

This change is Reviewable

@highfive
Copy link

Heads up! This PR modifies the following files:

  • @bholley: components/style/properties/longhand/font.mako.rs, components/style/properties/shorthand/font.mako.rs, components/style/properties/data.py
  • @emilio: components/style/properties/longhand/font.mako.rs, components/style/properties/shorthand/font.mako.rs, components/style/properties/data.py

@highfive highfive added the S-awaiting-review There is new code that needs to be reviewed. label May 12, 2017
@highfive
Copy link

warning Warning warning

  • These commits modify style code, but no tests are modified. Please consider adding a test!

@upsuper
Copy link
Contributor Author

upsuper commented May 12, 2017

r? @Manishearth

@highfive highfive assigned Manishearth and unassigned glennw May 12, 2017
@Manishearth
Copy link
Member

r+

@upsuper
Copy link
Contributor Author

upsuper commented May 12, 2017

@bors-servo r=Manishearth

@bors-servo
Copy link
Contributor

📌 Commit 0e09b81 has been approved by Manishearth

@highfive highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. labels May 12, 2017
@bors-servo
Copy link
Contributor

⌛ Testing commit 0e09b81 with merge f2e80f5...

bors-servo pushed a commit that referenced this pull request May 12, 2017
…hearth

Make font-feature-settings a subprop of font

<!-- Reviewable:start -->
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/16827)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

💔 Test failed - linux-dev

@highfive highfive added S-tests-failed The changes caused existing tests to fail. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels May 12, 2017
@Manishearth
Copy link
Member

error[E0308]: mismatched types
  --> /home/servo/buildbot/slave/linux-dev/build/tests/unit/style/parsing/font.rs:18:5
   |
18 |     assert_eq!(normal, normal_computed);
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected enum `style::properties::longhands::font_feature_settings::SpecifiedValue`, found enum `style::computed_values::computed_value::T`
   |
   = note: expected type `style::properties::longhands::font_feature_settings::SpecifiedValue`
              found type `style::computed_values::computed_value::T`
   = note: this error originates in a macro outside of the current crate

error[E0308]: mismatched types
  --> /home/servo/buildbot/slave/linux-dev/build/tests/unit/style/parsing/font.rs:30:5
   |
30 |     assert_eq!(on, on_computed);
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected enum `style::properties::longhands::font_feature_settings::SpecifiedValue`, found enum `style::computed_values::computed_value::T`
   |
   = note: expected type `style::properties::longhands::font_feature_settings::SpecifiedValue`
              found type `style::computed_values::computed_value::T`
   = note: this error originates in a macro outside of the current crate

error[E0308]: mismatched types
  --> /home/servo/buildbot/slave/linux-dev/build/tests/unit/style/parsing/font.rs:36:5
   |
36 |     assert_eq!(off, off_computed);
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected enum `style::properties::longhands::font_feature_settings::SpecifiedValue`, found enum `style::computed_values::computed_value::T`
   |
   = note: expected type `style::properties::longhands::font_feature_settings::SpecifiedValue`
              found type `style::computed_values::computed_value::T`
   = note: this error originates in a macro outside of the current crate

error[E0308]: mismatched types
  --> /home/servo/buildbot/slave/linux-dev/build/tests/unit/style/parsing/font.rs:42:5
   |
42 |     assert_eq!(no_value, no_value_computed);
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected enum `style::properties::longhands::font_feature_settings::SpecifiedValue`, found enum `style::computed_values::computed_value::T`
   |
   = note: expected type `style::properties::longhands::font_feature_settings::SpecifiedValue`
              found type `style::computed_values::computed_value::T`
   = note: this error originates in a macro outside of the current crate

error[E0308]: mismatched types
  --> /home/servo/buildbot/slave/linux-dev/build/tests/unit/style/parsing/font.rs:48:5
   |
48 |     assert_eq!(pos_integer, pos_integer_computed);
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected enum `style::properties::longhands::font_feature_settings::SpecifiedValue`, found enum `style::computed_values::computed_value::T`
   |
   = note: expected type `style::properties::longhands::font_feature_settings::SpecifiedValue`
              found type `style::computed_values::computed_value::T`
   = note: this error originates in a macro outside of the current crate

error[E0308]: mismatched types
  --> /home/servo/buildbot/slave/linux-dev/build/tests/unit/style/parsing/font.rs:55:5
   |
55 |     assert_eq!(multiple, multiple_computed);
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected enum `style::properties::longhands::font_feature_settings::SpecifiedValue`, found enum `style::computed_values::computed_value::T`
   |
   = note: expected type `style::properties::longhands::font_feature_settings::SpecifiedValue`
              found type `style::computed_values::computed_value::T`
   = note: this error originates in a macro outside of the current crate

error: aborting due to 6 previous errors


---- size_of::size_of_specified_values stdout ----
	thread 'size_of::size_of_specified_values' panicked at 'Your changes have increased the size of font-feature-settings SpecifiedValue to 32. The threshold is                         currently 24. SpecifiedValues affect size of PropertyDeclaration enum and                         increasing the size may negative affect style system performance. Please consider                         using `boxed="True"` in this longhand.', /home/servo/buildbot/slave/linux-dev/build/target/geckolib/debug/build/style-d6a6d23c5462c560/out/properties.rs:158832
stack backtrace:
   1:     0x7f56ad97fbfc - std::sys::imp::backtrace::tracing::imp::write::hf33ae72d0baa11ed
                        at /buildslave/rust-buildbot/slave/stable-dist-rustc-linux/build/src/libstd/sys/unix/backtrace/tracing/gcc_s.rs:42
   2:     0x7f56ad982d4e - std::panicking::default_hook::{{closure}}::h59672b733cc6a455
                        at /buildslave/rust-buildbot/slave/stable-dist-rustc-linux/build/src/libstd/panicking.rs:351
   3:     0x7f56ad9828f3 - std::panicking::default_hook::h1670459d2f3f8843
                        at /buildslave/rust-buildbot/slave/stable-dist-rustc-linux/build/src/libstd/panicking.rs:361
   4:     0x7f56ad9831eb - std::panicking::rust_panic_with_hook::hcf0ddb069e7beee7
                        at /buildslave/rust-buildbot/slave/stable-dist-rustc-linux/build/src/libstd/panicking.rs:555
   5:     0x7f56ad983084 - std::panicking::begin_panic::hd6eb68e27bdf6140
                        at /buildslave/rust-buildbot/slave/stable-dist-rustc-linux/build/src/libstd/panicking.rs:517
   6:     0x7f56ad982fa9 - std::panicking::begin_panic_fmt::hfea5965948b877f8
                        at /buildslave/rust-buildbot/slave/stable-dist-rustc-linux/build/src/libstd/panicking.rs:501
   7:     0x7f56ad9722fa - style::properties::test_size_of_specified_values::haa9b9346adcbf67b
                        at /home/servo/buildbot/slave/linux-dev/build/components/style/lib.rs:155
   8:     0x7f56ad93473c - stylo_tests::size_of::size_of_specified_values::he927dd7ae680b956
                        at /home/servo/buildbot/slave/linux-dev/build/tests/unit/stylo/size_of.rs:12
   9:     0x7f56ad942a8e - <F as test::FnBox<T>>::call_box::he0ddef732487ab41
                        at /buildslave/rust-buildbot/slave/stable-dist-rustc-linux/build/src/libtest/lib.rs:1366
                        at /buildslave/rust-buildbot/slave/stable-dist-rustc-linux/build/src/libtest/lib.rs:140
  10:     0x7f56ad98a0fa - __rust_maybe_catch_panic
                        at /buildslave/rust-buildbot/slave/stable-dist-rustc-linux/build/src/libpanic_unwind/lib.rs:98
  11:     0x7f56ad936e7a - std::panicking::try::do_call::h4652d5e10f648844
                        at /buildslave/rust-buildbot/slave/stable-dist-rustc-linux/build/src/libstd/panicking.rs:436
                        at /buildslave/rust-buildbot/slave/stable-dist-rustc-linux/build/src/libstd/panic.rs:361
                        at /buildslave/rust-buildbot/slave/stable-dist-rustc-linux/build/src/libtest/lib.rs:1311
                        at /buildslave/rust-buildbot/slave/stable-dist-rustc-linux/build/src/libstd/panic.rs:296
                        at /buildslave/rust-buildbot/slave/stable-dist-rustc-linux/build/src/libstd/panicking.rs:460
  12:     0x7f56ad98a0fa - __rust_maybe_catch_panic
                        at /buildslave/rust-buildbot/slave/stable-dist-rustc-linux/build/src/libpanic_unwind/lib.rs:98
  13:     0x7f56ad93daa6 - <F as alloc::boxed::FnBox<A>>::call_box::h274e3783c3771bc8
                        at /buildslave/rust-buildbot/slave/stable-dist-rustc-linux/build/src/libstd/panicking.rs:436
                        at /buildslave/rust-buildbot/slave/stable-dist-rustc-linux/build/src/libstd/panic.rs:361
                        at /buildslave/rust-buildbot/slave/stable-dist-rustc-linux/build/src/libstd/thread/mod.rs:357
                        at /buildslave/rust-buildbot/slave/stable-dist-rustc-linux/build/src/liballoc/boxed.rs:614
  14:     0x7f56ad982054 - std::sys::imp::thread::Thread::new::thread_start::he668872ac11287ba
                        at /buildslave/rust-buildbot/slave/stable-dist-rustc-linux/build/src/liballoc/boxed.rs:624
                        at /buildslave/rust-buildbot/slave/stable-dist-rustc-linux/build/src/libstd/sys_common/thread.rs:21
                        at /buildslave/rust-buildbot/slave/stable-dist-rustc-linux/build/src/libstd/sys/unix/thread.rs:84
  15:     0x7f56ad0dd183 - start_thread
  16:     0x7f56acbf4bec - clone
  17:                0x0 - <unknown>


failures:
    size_of::size_of_property_declaration
    size_of::size_of_specified_values

test result: FAILED. 1 passed; 2 failed; 0 ignored; 0 measured

@upsuper upsuper force-pushed the subprop-font-feature-settings branch from 0e09b81 to 075ce76 Compare May 13, 2017 06:59
@highfive highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-tests-failed The changes caused existing tests to fail. labels May 13, 2017
@upsuper
Copy link
Contributor Author

upsuper commented May 13, 2017

@bors-servo try

@bors-servo
Copy link
Contributor

⌛ Trying commit 075ce76 with merge d5525db...

bors-servo pushed a commit that referenced this pull request May 13, 2017
Make font-feature-settings a subprop of font

<!-- Reviewable:start -->
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/16827)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

💔 Test failed - linux-dev

@highfive highfive added the S-tests-failed The changes caused existing tests to fail. label May 13, 2017
@upsuper upsuper force-pushed the subprop-font-feature-settings branch from 075ce76 to 5b4f068 Compare May 13, 2017 07:26
@highfive highfive removed the S-tests-failed The changes caused existing tests to fail. label May 13, 2017
@upsuper
Copy link
Contributor Author

upsuper commented May 13, 2017

@bors-servo try

@bors-servo
Copy link
Contributor

⌛ Trying commit 5b4f068 with merge 2a97e5f...

bors-servo pushed a commit that referenced this pull request May 13, 2017
Make font-feature-settings a subprop of font

<!-- Reviewable:start -->
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/16827)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css, mac-rel-wpt1, mac-rel-wpt2, windows-msvc-dev
State: approved= try=True

@upsuper
Copy link
Contributor Author

upsuper commented May 13, 2017

@bors-servo r=Manishearth

@bors-servo
Copy link
Contributor

📌 Commit 5b4f068 has been approved by Manishearth

@highfive highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. labels May 13, 2017
@bors-servo
Copy link
Contributor

⌛ Testing commit 5b4f068 with merge e83e102...

bors-servo pushed a commit that referenced this pull request May 13, 2017
…hearth

Make font-feature-settings a subprop of font

<!-- Reviewable:start -->
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/16827)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css, mac-rel-wpt1, mac-rel-wpt2, windows-msvc-dev
Approved by: Manishearth
Pushing e83e102 to master...

@bors-servo bors-servo merged commit 5b4f068 into servo:master May 13, 2017
@highfive highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label May 13, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants