From c17fdaaa27255349fa21f1bd5b4e969ab1e1bf70 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 26 Aug 2019 23:18:46 +0000 Subject: [PATCH] Bug 1576703 - Update cbindgen. r=boris This cleans up the pattern of "Use a private dtor so that the helper functions do the right thing" by enabling it everywhere using: https://github.com/eqrion/cbindgen/pull/377 It also caught some uninitialized value issues. I think they're mostly harmless since we zero-initialize our structs: https://searchfox.org/mozilla-central/rev/325c1a707819602feff736f129cb36055ba6d94f/servo/components/style/properties/gecko.mako.rs#632 And since we override the clip rect, which is the other bit of code that was failing to build with this change. Differential Revision: https://phabricator.services.mozilla.com/D43491 --HG-- extra : moz-landing-system : lando --- build/moz.configure/bindgen.configure | 2 +- layout/style/FontFace.cpp | 2 +- layout/style/FontFaceSet.cpp | 2 +- layout/style/GeckoBindings.cpp | 4 +- layout/style/nsStyleStruct.cpp | 4 + .../components/style/values/computed/text.rs | 2 - .../style/values/generics/effects.rs | 2 - .../components/style/values/generics/grid.rs | 8 -- servo/components/style/values/generics/svg.rs | 5 - .../style/values/generics/transform.rs | 1 - servo/components/style/values/generics/url.rs | 2 - .../components/style/values/specified/font.rs | 2 - .../components/style/values/specified/list.rs | 2 - .../style/values/specified/motion.rs | 1 - .../style/values/specified/position.rs | 2 - .../components/style/values/specified/text.rs | 1 - servo/ports/geckolib/cbindgen.toml | 109 +----------------- taskcluster/ci/fetch/toolchains.yml | 4 +- taskcluster/ci/toolchain/cbindgen.yml | 2 +- 19 files changed, 15 insertions(+), 142 deletions(-) diff --git a/build/moz.configure/bindgen.configure b/build/moz.configure/bindgen.configure index 4df185e1a3fb..335768ffe27f 100644 --- a/build/moz.configure/bindgen.configure +++ b/build/moz.configure/bindgen.configure @@ -15,7 +15,7 @@ option(env='CBINDGEN', nargs=1, when=cbindgen_is_needed, def check_cbindgen_version(cbindgen, fatal=False): log.debug("trying cbindgen: %s" % cbindgen) - cbindgen_min_version = Version('0.9.0') + cbindgen_min_version = Version('0.9.1') # cbindgen x.y.z version = Version(check_cmd_output(cbindgen, '--version').strip().split(" ")[1]) diff --git a/layout/style/FontFace.cpp b/layout/style/FontFace.cpp index e58e9f8dd833..9d5f5e73130a 100644 --- a/layout/style/FontFace.cpp +++ b/layout/style/FontFace.cpp @@ -629,7 +629,7 @@ Maybe FontFace::GetFontStretch() const { } Maybe FontFace::GetFontStyle() const { - StyleComputedFontStyleDescriptor descriptor; + auto descriptor = StyleComputedFontStyleDescriptor::Normal(); if (!Servo_FontFaceRule_GetFontStyle(GetData(), &descriptor)) { return Nothing(); } diff --git a/layout/style/FontFaceSet.cpp b/layout/style/FontFaceSet.cpp index a8430332e24b..d05492c97e73 100644 --- a/layout/style/FontFaceSet.cpp +++ b/layout/style/FontFaceSet.cpp @@ -205,7 +205,7 @@ void FontFaceSet::ParseFontShorthandForMatching( const nsAString& aFont, RefPtr& aFamilyList, FontWeight& aWeight, FontStretch& aStretch, FontSlantStyle& aStyle, ErrorResult& aRv) { - StyleComputedFontStyleDescriptor style; + auto style = StyleComputedFontStyleDescriptor::Normal(); float stretch; float weight; diff --git a/layout/style/GeckoBindings.cpp b/layout/style/GeckoBindings.cpp index 88eb5a4d53c1..af01c8683624 100644 --- a/layout/style/GeckoBindings.cpp +++ b/layout/style/GeckoBindings.cpp @@ -1152,7 +1152,9 @@ void Gecko_CopyImageValueFrom(nsStyleImage* aImage, void Gecko_InitializeImageCropRect(nsStyleImage* aImage) { MOZ_ASSERT(aImage); - aImage->SetCropRect(MakeUnique()); + auto zero = StyleNumberOrPercentage::Number(0); + aImage->SetCropRect(MakeUnique( + nsStyleImage::CropRect{zero, zero, zero, zero})); } void Gecko_SetCursorArrayLength(nsStyleUI* aStyleUI, size_t aLen) { diff --git a/layout/style/nsStyleStruct.cpp b/layout/style/nsStyleStruct.cpp index 7d869e547bf2..5870c9ecff9c 100644 --- a/layout/style/nsStyleStruct.cpp +++ b/layout/style/nsStyleStruct.cpp @@ -542,6 +542,7 @@ nsChangeHint nsStyleOutline::CalcDifference( nsStyleList::nsStyleList(const Document& aDocument) : mListStylePosition(NS_STYLE_LIST_STYLE_POSITION_OUTSIDE), mQuotes(StyleQuotes::Auto()), + mImageRegion(StyleClipRectOrAuto::Auto()), mMozListReversed(StyleMozListReversed::False) { MOZ_COUNT_CTOR(nsStyleList); MOZ_ASSERT(NS_IsMainThread()); @@ -2698,6 +2699,9 @@ nsStyleDisplay::nsStyleDisplay(const Document& aDocument) mScrollSnapType( {StyleScrollSnapAxis::Both, StyleScrollSnapStrictness::None}), mLineClamp(0), + mRotate(StyleRotate::None()), + mTranslate(StyleTranslate::None()), + mScale(StyleScale::None()), mBackfaceVisibility(NS_STYLE_BACKFACE_VISIBILITY_VISIBLE), mTransformStyle(NS_STYLE_TRANSFORM_STYLE_FLAT), mTransformBox(StyleGeometryBox::BorderBox), diff --git a/servo/components/style/values/computed/text.rs b/servo/components/style/values/computed/text.rs index bdb2ac507bc7..4cccd248d1c3 100644 --- a/servo/components/style/values/computed/text.rs +++ b/servo/components/style/values/computed/text.rs @@ -198,8 +198,6 @@ impl TextDecorationsInEffect { } /// Computed value for the text-emphasis-style property -/// -/// cbindgen:derive-tagged-enum-copy-constructor=true #[derive(Clone, Debug, MallocSizeOf, PartialEq, ToCss, ToResolvedValue)] #[allow(missing_docs)] #[repr(C, u8)] diff --git a/servo/components/style/values/generics/effects.rs b/servo/components/style/values/generics/effects.rs index 724f484f2834..dd9da8759bea 100644 --- a/servo/components/style/values/generics/effects.rs +++ b/servo/components/style/values/generics/effects.rs @@ -34,8 +34,6 @@ pub struct GenericBoxShadow { pub use self::GenericBoxShadow as BoxShadow; /// A generic value for a single `filter`. -/// -/// cbindgen:derive-tagged-enum-copy-constructor=true #[cfg_attr(feature = "servo", derive(Deserialize, Serialize))] #[animation(no_bound(U))] #[derive( diff --git a/servo/components/style/values/generics/grid.rs b/servo/components/style/values/generics/grid.rs index b9e173ea50f3..29f634d10fec 100644 --- a/servo/components/style/values/generics/grid.rs +++ b/servo/components/style/values/generics/grid.rs @@ -185,8 +185,6 @@ impl Parse for GridLine { /// avoid re-implementing it for the computed type. /// /// -/// -/// cbindgen:derive-tagged-enum-copy-constructor=true #[derive( Animate, Clone, @@ -230,8 +228,6 @@ impl TrackBreadth { /// generic only to avoid code bloat. It only takes `` /// /// -/// -/// cbindgen:derive-tagged-enum-copy-constructor=true #[derive( Clone, Debug, @@ -494,8 +490,6 @@ impl ToCss for TrackRepeat { } /// Track list values. Can be or -/// -/// cbindgen:derive-tagged-enum-copy-constructor=true #[derive( Animate, Clone, @@ -729,8 +723,6 @@ impl ToCss for LineNameList { } /// Variants for ` | ` -/// -/// cbindgen:derive-tagged-enum-copy-constructor=true #[derive( Animate, Clone, diff --git a/servo/components/style/values/generics/svg.rs b/servo/components/style/values/generics/svg.rs index 4a1fc42551d5..82183c305645 100644 --- a/servo/components/style/values/generics/svg.rs +++ b/servo/components/style/values/generics/svg.rs @@ -9,7 +9,6 @@ use cssparser::Parser; use style_traits::ParseError; /// The fallback of an SVG paint server value. -/// cbindgen:derive-tagged-enum-copy-constructor=true #[derive( Animate, Clone, @@ -43,8 +42,6 @@ pub use self::GenericSVGPaintFallback as SVGPaintFallback; /// An SVG paint value /// /// -/// -/// cbindgen:derive-tagged-enum-copy-constructor=true #[animation(no_bound(Url))] #[derive( Animate, @@ -84,8 +81,6 @@ impl Default for SVGPaint { /// /// Whereas the spec only allows PaintServer to have a fallback, Gecko lets the /// context properties have a fallback as well. -/// -/// cbindgen:derive-tagged-enum-copy-constructor=true #[animation(no_bound(U))] #[derive( Animate, diff --git a/servo/components/style/values/generics/transform.rs b/servo/components/style/values/generics/transform.rs index 8dbbdfcd1689..4a8dc7f63fac 100644 --- a/servo/components/style/values/generics/transform.rs +++ b/servo/components/style/values/generics/transform.rs @@ -151,7 +151,6 @@ fn is_same(x: &N, y: &N) -> bool { )] #[repr(C, u8)] /// A single operation in the list of a `transform` value -/// cbindgen:derive-tagged-enum-copy-constructor=true pub enum GenericTransformOperation where Angle: Zero, diff --git a/servo/components/style/values/generics/url.rs b/servo/components/style/values/generics/url.rs index 1f2710330365..46ed453e82d3 100644 --- a/servo/components/style/values/generics/url.rs +++ b/servo/components/style/values/generics/url.rs @@ -5,8 +5,6 @@ //! Generic types for url properties. /// An image url or none, used for example in list-style-image -/// -/// cbindgen:derive-tagged-enum-copy-constructor=true #[derive( Animate, Clone, diff --git a/servo/components/style/values/specified/font.rs b/servo/components/style/values/specified/font.rs index 9e680ec77582..45af8df23bff 100644 --- a/servo/components/style/values/specified/font.rs +++ b/servo/components/style/values/specified/font.rs @@ -1091,8 +1091,6 @@ bitflags! { )] #[repr(C, u8)] /// Set of variant alternates -/// -/// cbindgen:derive-tagged-enum-copy-constructor=true pub enum VariantAlternates { /// Enables display of stylistic alternates #[css(function)] diff --git a/servo/components/style/values/specified/list.rs b/servo/components/style/values/specified/list.rs index 3946d78652ef..084889a3a223 100644 --- a/servo/components/style/values/specified/list.rs +++ b/servo/components/style/values/specified/list.rs @@ -126,8 +126,6 @@ pub struct QuoteList( /// Specified and computed `quotes` property: `auto`, `none`, or a list /// of characters. -/// -/// cbindgen:derive-tagged-enum-copy-constructor=true #[derive( Clone, Debug, diff --git a/servo/components/style/values/specified/motion.rs b/servo/components/style/values/specified/motion.rs index 97609ea20b01..8d6f7809fdbe 100644 --- a/servo/components/style/values/specified/motion.rs +++ b/servo/components/style/values/specified/motion.rs @@ -15,7 +15,6 @@ use style_traits::{ParseError, StyleParseErrorKind}; /// The offset-path value. /// /// https://drafts.fxtf.org/motion-1/#offset-path-property -/// cbindgen:derive-tagged-enum-copy-constructor=true #[derive( Animate, Clone, diff --git a/servo/components/style/values/specified/position.rs b/servo/components/style/values/specified/position.rs index ce3bab1047be..d3476bb9f65c 100644 --- a/servo/components/style/values/specified/position.rs +++ b/servo/components/style/values/specified/position.rs @@ -707,8 +707,6 @@ fn is_name_code_point(c: char) -> bool { /// The syntax of this property also provides a visualization of the structure /// of the grid, making the overall layout of the grid container easier to /// understand. -/// -/// cbindgen:derive-tagged-enum-copy-constructor=true #[repr(C, u8)] #[derive( Clone, diff --git a/servo/components/style/values/specified/text.rs b/servo/components/style/values/specified/text.rs index ebc8e8f86ee1..1505192f86f7 100644 --- a/servo/components/style/values/specified/text.rs +++ b/servo/components/style/values/specified/text.rs @@ -132,7 +132,6 @@ impl ToComputedValue for LineHeight { } /// A generic value for the `text-overflow` property. -/// cbindgen:derive-tagged-enum-copy-constructor=true #[derive(Clone, Debug, Eq, MallocSizeOf, PartialEq, SpecifiedValueInfo, ToCss, ToShmem)] #[repr(C, u8)] pub enum TextOverflowSide { diff --git a/servo/ports/geckolib/cbindgen.toml b/servo/ports/geckolib/cbindgen.toml index d289c2710d9a..1b65f5904bb8 100644 --- a/servo/ports/geckolib/cbindgen.toml +++ b/servo/ports/geckolib/cbindgen.toml @@ -48,6 +48,8 @@ bitflags = true derive_helper_methods = true derive_const_casts = true derive_tagged_enum_destructor = true +derive_tagged_enum_copy_constructor = true +private_default_tagged_enum_constructor = true cast_assert_name = "MOZ_ASSERT" [export] @@ -331,15 +333,6 @@ renaming_overrides_prefixing = true inline bool IsNone() const; """ -"Quotes" = """ - private: - // Private default constructor without initialization so that the helper - // constructor functions still work as expected. They take care of - // initializing the fields properly. - StyleQuotes() {} - public: -""" - # TODO(emilio): Add hooks to cbindgen to be able to generate MOZ_MUST_USE_TYPE # or MOZ_MUST_USE on the functions. "Owned" = """ @@ -519,47 +512,12 @@ renaming_overrides_prefixing = true bool HasPercent() const; """ -"GenericTransformOperation" = """ - private: - // Private default constructor without initialization so that the helper - // constructor functions still work as expected. They take care of - // initializing the fields properly. - StyleGenericTransformOperation() {} - public: -""" - -"GenericUrlOrNone" = """ - private: - // Private default constructor without initialization so that the helper - // constructor functions still work as expected. They take care of - // initializing the fields properly. - StyleGenericUrlOrNone() {} - public: -""" "Angle" = """ inline static StyleAngle Zero(); inline float ToDegrees() const; inline double ToRadians() const; """ -"OffsetPath" = """ - private: - // Private default constructor without initialization so that the helper - // constructor functions still work as expected. They take care of - // initializing the fields properly. - StyleOffsetPath() {} - public: -""" - -"TextOverflowSide" = """ - private: - // Private default constructor without initialization so that the helper - // constructor functions still work as expected. They take care of - // initializing the fields properly. - StyleTextOverflowSide() {} - public: -""" - "TextOverflow" = """ StyleTextOverflow() : first(StyleTextOverflowSide::Clip()), @@ -618,15 +576,6 @@ renaming_overrides_prefixing = true bool IsOpaque() const; """ -"GridTemplateAreas" = """ - private: - // Private default constructor without initialization so that the helper - // constructor functions still work as expected. They take care of - // initializing the fields properly. - StyleGridTemplateAreas() {} - public: -""" - "GenericGridLine" = """ // Returns the `auto` value. inline StyleGenericGridLine(); @@ -636,52 +585,16 @@ renaming_overrides_prefixing = true """ "GenericTrackBreadth" = """ - private: - // Private default constructor without initialization so that the helper - // constructor functions still work as expected. They take care of - // initializing the fields properly. - StyleGenericTrackBreadth() {} - public: inline bool HasPercent() const; """ "GenericTrackSize" = """ - private: - // Private default constructor without initialization so that the helper - // constructor functions still work as expected. They take care of - // initializing the fields properly. - StyleGenericTrackSize() {} - public: // Implemented in nsGridContainerFrame.cpp inline const StyleGenericTrackBreadth& GetMin() const; inline const StyleGenericTrackBreadth& GetMax() const; """ -"GenericSVGPaintKind" = """ - private: - // Private default constructor without initialization so that the helper - // constructor functions still work as expected. They take care of - // initializing the fields properly. - StyleGenericSVGPaintKind() {} - public: -""" - -"GenericSVGPaintFallback" = """ - private: - // Private default constructor without initialization so that the helper - // constructor functions still work as expected. They take care of - // initializing the fields properly. - StyleGenericSVGPaintFallback() {} - public: -""" - "GenericGridTemplateComponent" = """ - private: - // Private default constructor without initialization so that the helper - // constructor functions still work as expected. They take care of - // initializing the fields properly. - StyleGenericGridTemplateComponent() {} - public: inline Maybe RepeatAutoIndex() const; inline const StyleGenericTrackRepeat* GetRepeatAutoValue() const; inline bool HasRepeatAuto() const; @@ -689,24 +602,6 @@ renaming_overrides_prefixing = true inline Span> TrackListValues() const; """ -"TextEmphasisStyle" = """ - private: - // Private default constructor without initialization so that the helper - // constructor functions still work as expected. They take care of - // initializing the fields properly. - StyleTextEmphasisStyle() {} - public: -""" - -"VariantAlternates" = """ - private: - // Private default constructor without initialization so that the helper - // constructor functions still work as expected. They take care of - // initializing the fields properly. - StyleVariantAlternates() {} - public: -""" - "GenericClipRect" = """ // Get the layout rect, replacing auto right / bottom values for aAutoSize. inline nsRect ToLayoutRect(nscoord aAutoSize = NS_MAXSIZE) const; diff --git a/taskcluster/ci/fetch/toolchains.yml b/taskcluster/ci/fetch/toolchains.yml index 8ac6e708ba42..1ad79cee5f68 100644 --- a/taskcluster/ci/fetch/toolchains.yml +++ b/taskcluster/ci/fetch/toolchains.yml @@ -256,12 +256,12 @@ wine-3.0.3: sig-url: "{url}.sign" key-path: build/unix/build-gcc/DA23579A74D4AD9AF9D3F945CEFAC8EAAF17519D.key -cbindgen-0.9.0: +cbindgen-0.9.1: description: cbindgen source code fetch: type: git repo: https://github.com/eqrion/cbindgen - revision: e19526e00b3fe6921b881682147a1fe5d6b64124 + revision: 8e4db4c17fbdc0cfa9b98cfe9d47ca6263858def cctools-port: description: cctools-port source code diff --git a/taskcluster/ci/toolchain/cbindgen.yml b/taskcluster/ci/toolchain/cbindgen.yml index c1c6c3eadb4a..9891b6f74e2f 100644 --- a/taskcluster/ci/toolchain/cbindgen.yml +++ b/taskcluster/ci/toolchain/cbindgen.yml @@ -14,7 +14,7 @@ job-defaults: fetch: # If you update this, make sure to update the minimum version in # build/moz.configure/bindgen.configure as well. - - cbindgen-0.9.0 + - cbindgen-0.9.1 linux64-cbindgen: treeherder: