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

Simplify bitfields #39485

Closed
wants to merge 3 commits into from
Closed

Conversation

NickGerleman
Copy link
Contributor

Summary:
These were previously packed into structs to allow zero initializing all the flags at once without needing a ctor. C++ 20 has in-class member initializer support for bitfields, which makes these look more like normal member variables.

Setting enum values is a bit jank right now, due to relying on C enums which are efftively int32_t, along with GCC -Wconversion being a bit aggressive in needing to explicitly mask. I have some ideas to fix this later (e.g. using scoped enums internally).

Differential Revision: D49265967

@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. p: Facebook Partner: Facebook Partner labels Sep 15, 2023
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D49265967

@analysis-bot
Copy link

analysis-bot commented Sep 15, 2023

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 8,341,809 -83
android hermes armeabi-v7a n/a --
android hermes x86 n/a --
android hermes x86_64 n/a --
android jsc arm64-v8a 9,580,678 -35
android jsc armeabi-v7a n/a --
android jsc x86 n/a --
android jsc x86_64 n/a --

Base commit: 0ab8b40
Branch: main

@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D49265967

NickGerleman added a commit to NickGerleman/react-native that referenced this pull request Sep 16, 2023
Summary:
Pull Request resolved: facebook#39485

X-link: facebook/yoga#1393

These were previously packed into structs to allow zero initializing all the flags at once without needing a ctor. C++ 20 has in-class member initializer support for bitfields, which makes these look more like normal member variables.

Setting enum values is a bit jank right now, due to relying on C enums which are efftively int32_t, along with GCC `-Wconversion` being a bit aggressive in needing to explicitly mask. I have some ideas to fix this later (e.g. using scoped enums internally).

Differential Revision: D49265967

fbshipit-source-id: 7aeb613352943f57f8a2f8a7274514996b0a0917
NickGerleman added a commit to NickGerleman/yoga that referenced this pull request Sep 16, 2023
Summary:
X-link: facebook/react-native#39485

Pull Request resolved: facebook#1393

These were previously packed into structs to allow zero initializing all the flags at once without needing a ctor. C++ 20 has in-class member initializer support for bitfields, which makes these look more like normal member variables.

Setting enum values is a bit jank right now, due to relying on C enums which are efftively int32_t, along with GCC `-Wconversion` being a bit aggressive in needing to explicitly mask. I have some ideas to fix this later (e.g. using scoped enums internally).

Differential Revision: D49265967

fbshipit-source-id: f72f9dec4caf0acaccf49902f2d3537187445fbe
NickGerleman added a commit to NickGerleman/yoga that referenced this pull request Sep 18, 2023
Summary:
X-link: facebook/react-native#39485

Pull Request resolved: facebook#1393

These were previously packed into structs to allow zero initializing all the flags at once without needing a ctor. C++ 20 has in-class member initializer support for bitfields, which makes these look more like normal member variables.

Setting enum values is a bit jank right now, due to relying on C enums which are efftively int32_t, along with GCC `-Wconversion` being a bit aggressive in needing to explicitly mask. I have some ideas to fix this later (e.g. using scoped enums internally).

Differential Revision: D49265967

fbshipit-source-id: 0a5c7a24dc0eade1ca037f0c8660a9e442da198f
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D49265967

NickGerleman added a commit to NickGerleman/react-native that referenced this pull request Sep 18, 2023
Summary:
Pull Request resolved: facebook#39485

X-link: facebook/yoga#1393

These were previously packed into structs to allow zero initializing all the flags at once without needing a ctor. C++ 20 has in-class member initializer support for bitfields, which makes these look more like normal member variables.

Setting enum values is a bit jank right now, due to relying on C enums which are efftively int32_t, along with GCC `-Wconversion` being a bit aggressive in needing to explicitly mask. I have some ideas to fix this later (e.g. using scoped enums internally).

Differential Revision: D49265967

fbshipit-source-id: adaa2973d671f6fbd79ffc063f572fb9c725b1ba
NickGerleman added a commit to NickGerleman/yoga that referenced this pull request Sep 18, 2023
Summary:
X-link: facebook/react-native#39485

Pull Request resolved: facebook#1393

These were previously packed into structs to allow zero initializing all the flags at once without needing a ctor. C++ 20 has in-class member initializer support for bitfields, which makes these look more like normal member variables.

Setting enum values is a bit jank right now, due to relying on C enums which are efftively int32_t, along with GCC `-Wconversion` being a bit aggressive in needing to explicitly mask. I have some ideas to fix this later (e.g. using scoped enums internally).

Differential Revision: D49265967

fbshipit-source-id: 2fa14d74f60cafcbbba9956cd6f9ce069a9b1c7c
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D49265967

NickGerleman added a commit to NickGerleman/react-native that referenced this pull request Sep 18, 2023
Summary:
Pull Request resolved: facebook#39485

X-link: facebook/yoga#1393

These were previously packed into structs to allow zero initializing all the flags at once without needing a ctor. C++ 20 has in-class member initializer support for bitfields, which makes these look more like normal member variables.

Setting enum values is a bit jank right now, due to relying on C enums which are efftively int32_t, along with GCC `-Wconversion` being a bit aggressive in needing to explicitly mask. I have some ideas to fix this later (e.g. using scoped enums internally).

Differential Revision: D49265967

fbshipit-source-id: c4c8a9392f68c9b398827e50880fed400554b738
NickGerleman and others added 3 commits September 17, 2023 20:52
Summary: Changelog: [Internal]

Differential Revision: D49360870

fbshipit-source-id: be3517b8620b1f018b2c46ba8aa5d4e8fe6f2c8e
Summary:
X-link: facebook/yoga#1382

Pull Request resolved: facebook#39437

Have been running into places where C++ 20 makes life easier for use like `std::bit_cast` (that one is easy to polyfill), in-class member initializer support for bitfields, designated initializers, defaulted comparison operator, concepts instead of SFINAE, and probably more.

Our other infra is in the process of making this jump, or already has. This tests it out everywhere, across the various reference builds, to see if we have any issues.

This is a bit more aggressive than I had previously communicated, but n - 1 is going to be a better long term place than n - 2.

Accounting for bundled STL, and using `bit_cast` as a reference feature, I think this means we require one of:
1. GCC 11+ (~2.5 years old)
1. Clang 14 (~2.5 years old)
1. VS 16.11 (~2 years old)

On mobile, the Clang 14 requirement translates to needing one of:
1. NDK 25 (~1 year old)
1. XCode 14.2.0 (~1 year old)

https://en.cppreference.com/w/cpp/compiler_support/20

I think it is likely everything will be buildable for a while under Clang 10 (released 3.5 years ago), GCC 11 (releaseed 3.5 years ago), and .

Anyone needing support for older C++ versions can lag behind on more recent changes. E.g. Yoga 2.0 supports C++ 14.

Differential Revision: https://internalfb.com/D49261607

fbshipit-source-id: 0fc64cea8f4d6268f6cce3432f8d92961a0eaade
Summary:
Pull Request resolved: facebook#39485

X-link: facebook/yoga#1393

These were previously packed into structs to allow zero initializing all the flags at once without needing a ctor. C++ 20 has in-class member initializer support for bitfields, which makes these look more like normal member variables.

Setting enum values is a bit jank right now, due to relying on C enums which are efftively int32_t, along with GCC `-Wconversion` being a bit aggressive in needing to explicitly mask. I have some ideas to fix this later (e.g. using scoped enums internally).

Differential Revision: D49265967

fbshipit-source-id: 787382a31cadf37aece0842c8788cfd51ba2f055
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D49265967

NickGerleman added a commit to NickGerleman/yoga that referenced this pull request Sep 18, 2023
Summary:
X-link: facebook/react-native#39485

Pull Request resolved: facebook#1393

These were previously packed into structs to allow zero initializing all the flags at once without needing a ctor. C++ 20 has in-class member initializer support for bitfields, which makes these look more like normal member variables.

Setting enum values is a bit jank right now, due to relying on C enums which are efftively int32_t, along with GCC `-Wconversion` being a bit aggressive in needing to explicitly mask. I have some ideas to fix this later (e.g. using scoped enums internally).

Differential Revision: D49265967

fbshipit-source-id: f368d342d7c2a18b5b61469ded5dd6e1b2b939f5
@github-actions
Copy link

Warnings
⚠️ One hour and a half have passed and the E2E jobs haven't finished yet.

Generated by 🚫 dangerJS against 1c0ea92

NickGerleman added a commit to NickGerleman/yoga that referenced this pull request Sep 19, 2023
Summary:
X-link: facebook/react-native#39485

Pull Request resolved: facebook#1393

These were previously packed into structs to allow zero initializing all the flags at once without needing a ctor. C++ 20 has in-class member initializer support for bitfields, which makes these look more like normal member variables.

Setting enum values is a bit jank right now, due to relying on C enums which are effectively int32_t, along with GCC `-Wconversion` being a bit aggressive in needing to explicitly mask. I have some ideas to fix this later (e.g. using scoped enums internally).

[General][Breaking] - Require C++ 20 when including renderer headers

Reviewed By: sammy-SC

Differential Revision: D49265967

fbshipit-source-id: 6eddef135c0105e6f3d535849091550ffc5c05e7
NickGerleman added a commit to NickGerleman/yoga that referenced this pull request Sep 19, 2023
Summary:
X-link: facebook/react-native#39485

Pull Request resolved: facebook#1393

These were previously packed into structs to allow zero initializing all the flags at once without needing a ctor. C++ 20 has in-class member initializer support for bitfields, which makes these look more like normal member variables.

Setting enum values is a bit jank right now, due to relying on C enums which are effectively int32_t, along with GCC `-Wconversion` being a bit aggressive in needing to explicitly mask. I have some ideas to fix this later (e.g. using scoped enums internally).

[General][Breaking] - Require C++ 20 when including renderer headers

Reviewed By: sammy-SC

Differential Revision: D49265967

fbshipit-source-id: 21f3e852510d21b7c3db184eb5886a808474aade
NickGerleman added a commit to NickGerleman/yoga that referenced this pull request Sep 19, 2023
Summary:
X-link: facebook/react-native#39485

Pull Request resolved: facebook#1393

These were previously packed into structs to allow zero initializing all the flags at once without needing a ctor. C++ 20 has in-class member initializer support for bitfields, which makes these look more like normal member variables.

Setting enum values is a bit jank right now, due to relying on C enums which are effectively int32_t, along with GCC `-Wconversion` being a bit aggressive in needing to explicitly mask. I have some ideas to fix this later (e.g. using scoped enums internally).

[General][Breaking] - Require C++ 20 when including renderer headers

Reviewed By: sammy-SC

Differential Revision: D49265967

fbshipit-source-id: b4d23585831f9db16ad32e58189843ade6408427
NickGerleman added a commit to NickGerleman/yoga that referenced this pull request Sep 19, 2023
Summary:
X-link: facebook/react-native#39485

Pull Request resolved: facebook#1393

These were previously packed into structs to allow zero initializing all the flags at once without needing a ctor. C++ 20 has in-class member initializer support for bitfields, which makes these look more like normal member variables.

Setting enum values is a bit jank right now, due to relying on C enums which are effectively int32_t, along with GCC `-Wconversion` being a bit aggressive in needing to explicitly mask. I have some ideas to fix this later (e.g. using scoped enums internally).

[General][Breaking] - Require C++ 20 when including renderer headers

Reviewed By: sammy-SC

Differential Revision: D49265967

fbshipit-source-id: 2a5b07f286cd7704594ba5ce92a3d29b734235b0
NickGerleman added a commit to NickGerleman/yoga that referenced this pull request Sep 19, 2023
Summary:
X-link: facebook/react-native#39485

Pull Request resolved: facebook#1393

These were previously packed into structs to allow zero initializing all the flags at once without needing a ctor. C++ 20 has in-class member initializer support for bitfields, which makes these look more like normal member variables.

Setting enum values is a bit jank right now, due to relying on C enums which are effectively int32_t, along with GCC `-Wconversion` being a bit aggressive in needing to explicitly mask. I have some ideas to fix this later (e.g. using scoped enums internally).

[General][Breaking] - Require C++ 20 when including renderer headers

Reviewed By: sammy-SC

Differential Revision: D49265967

fbshipit-source-id: eda30a80d4948b950a11b52f63b9036479df05aa
NickGerleman added a commit to NickGerleman/yoga that referenced this pull request Sep 19, 2023
Summary:
X-link: facebook/react-native#39485

Pull Request resolved: facebook#1393

These were previously packed into structs to allow zero initializing all the flags at once without needing a ctor. C++ 20 has in-class member initializer support for bitfields, which makes these look more like normal member variables.

Setting enum values is a bit jank right now, due to relying on C enums which are effectively int32_t, along with GCC `-Wconversion` being a bit aggressive in needing to explicitly mask. I have some ideas to fix this later (e.g. using scoped enums internally).

[General][Breaking] - Require C++ 20 when including renderer headers

Reviewed By: sammy-SC

Differential Revision: D49265967

fbshipit-source-id: 4e5a6de77f7b8bab07004b3c5886923fa9567290
NickGerleman added a commit to NickGerleman/yoga that referenced this pull request Sep 19, 2023
Summary:
X-link: facebook/react-native#39485

Pull Request resolved: facebook#1393

These were previously packed into structs to allow zero initializing all the flags at once without needing a ctor. C++ 20 has in-class member initializer support for bitfields, which makes these look more like normal member variables.

Setting enum values is a bit jank right now, due to relying on C enums which are effectively int32_t, along with GCC `-Wconversion` being a bit aggressive in needing to explicitly mask. I have some ideas to fix this later (e.g. using scoped enums internally).

[General][Breaking] - Require C++ 20 when including renderer headers

Reviewed By: sammy-SC

Differential Revision: D49265967

fbshipit-source-id: a2d7ff728ffddb5b4bf9beccd08078247414fbca
NickGerleman added a commit to NickGerleman/yoga that referenced this pull request Sep 19, 2023
Summary:
X-link: facebook/react-native#39485

Pull Request resolved: facebook#1393

These were previously packed into structs to allow zero initializing all the flags at once without needing a ctor. C++ 20 has in-class member initializer support for bitfields, which makes these look more like normal member variables.

Setting enum values is a bit jank right now, due to relying on C enums which are effectively int32_t, along with GCC `-Wconversion` being a bit aggressive in needing to explicitly mask. I have some ideas to fix this later (e.g. using scoped enums internally).

bypass-github-export-checks

Changelog:
[General][Breaking] - Require C++ 20 when including renderer headers

Differential Revision: https://internalfb.com/D49265967

fbshipit-source-id: 8cd943ca329d28d6b4336bf74734bda53916c1ea
NickGerleman added a commit to NickGerleman/yoga that referenced this pull request Sep 19, 2023
Summary:
X-link: facebook/react-native#39485

Pull Request resolved: facebook#1393

These were previously packed into structs to allow zero initializing all the flags at once without needing a ctor. C++ 20 has in-class member initializer support for bitfields, which makes these look more like normal member variables.

Setting enum values is a bit jank right now, due to relying on C enums which are effectively int32_t, along with GCC `-Wconversion` being a bit aggressive in needing to explicitly mask. I have some ideas to fix this later (e.g. using scoped enums internally).

bypass-github-export-checks

Changelog:
[General][Breaking] - Require C++ 20 when including renderer headers

Reviewed By: sammy-SC

Differential Revision: D49265967

fbshipit-source-id: 7aba8d5c0c296fec571f2e29279648b93eacfbab
facebook-github-bot pushed a commit to facebook/yoga that referenced this pull request Sep 19, 2023
Summary:
X-link: facebook/react-native#39485

Pull Request resolved: #1393

These were previously packed into structs to allow zero initializing all the flags at once without needing a ctor. C++ 20 has in-class member initializer support for bitfields, which makes these look more like normal member variables.

Setting enum values is a bit jank right now, due to relying on C enums which are effectively int32_t, along with GCC `-Wconversion` being a bit aggressive in needing to explicitly mask. I have some ideas to fix this later (e.g. using scoped enums internally).

bypass-github-export-checks

Changelog:
[General][Breaking] - Require C++ 20 when including renderer headers

Reviewed By: sammy-SC

Differential Revision: D49265967

fbshipit-source-id: 6ab935a866196df06e742c821f3af88eb4d18e1a
facebook-github-bot pushed a commit to facebook/litho that referenced this pull request Sep 19, 2023
Summary:
X-link: facebook/react-native#39485

X-link: facebook/yoga#1393

These were previously packed into structs to allow zero initializing all the flags at once without needing a ctor. C++ 20 has in-class member initializer support for bitfields, which makes these look more like normal member variables.

Setting enum values is a bit jank right now, due to relying on C enums which are effectively int32_t, along with GCC `-Wconversion` being a bit aggressive in needing to explicitly mask. I have some ideas to fix this later (e.g. using scoped enums internally).

bypass-github-export-checks

Changelog:
[General][Breaking] - Require C++ 20 when including renderer headers

Reviewed By: sammy-SC

Differential Revision: D49265967

fbshipit-source-id: 6ab935a866196df06e742c821f3af88eb4d18e1a
@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label Sep 19, 2023
@facebook-github-bot
Copy link
Contributor

This pull request has been merged in 3b5bea0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported Merged This PR has been merged. p: Facebook Partner: Facebook Partner
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants