Skip to content

Adjust RawPropsPropNameLength's type to account for increased number of props #39008

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

Closed
wants to merge 1 commit into from

Conversation

vincentriemer
Copy link
Contributor

Summary:
Changelog: [Internal] - Adjust RawPropsPropNameLength's type to account for increased number of props

While investigating why we needed to back out D48288752 I discovered that the root cause was that the items_ vector in RawProsKeyMap was now a size greater than 255 which becomes an issue because items_'s indices are statically cast to RawPropsPropNameLength (previously alias to uint8_t).

This diff updates RawPropsPropNameLength to be an alias to uint16_t so the current issue is resolved as well as adding an assert to ensure (however unlikely) that this happens again.

Differential Revision: D48331909

@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 Aug 14, 2023
@facebook-github-bot
Copy link
Contributor

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

@facebook-github-bot
Copy link
Contributor

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

vincentriemer added a commit to vincentriemer/react-native that referenced this pull request Aug 14, 2023
…of props (facebook#39008)

Summary:
Pull Request resolved: facebook#39008

Changelog: [Internal] - Adjust RawPropsPropNameLength's type to account for increased number of props

While investigating why we needed to back out D48288752 I discovered that the root cause was that the `items_` vector in `RawProsKeyMap` was now a size greater than 255 which becomes an issue because `items_`'s indices are statically cast to `RawPropsPropNameLength` (previously alias to `uint8_t`).

This diff updates `RawPropsPropNameLength` to be an alias to `uint16_t` so the current issue is resolved as well as adding an assert to ensure (however unlikely) that this happens again.

Differential Revision: D48331909

fbshipit-source-id: e89f4c1dce36e63dc9c395eca4ff47b18b5f5e12
@analysis-bot
Copy link

analysis-bot commented Aug 14, 2023

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 8,946,638 -134
android hermes armeabi-v7a n/a --
android hermes x86 n/a --
android hermes x86_64 n/a --
android jsc arm64-v8a 9,540,277 -92
android jsc armeabi-v7a n/a --
android jsc x86 n/a --
android jsc x86_64 n/a --

Base commit: b012027
Branch: main

@facebook-github-bot
Copy link
Contributor

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

vincentriemer added a commit to vincentriemer/react-native that referenced this pull request Aug 15, 2023
…of props (facebook#39008)

Summary:
Pull Request resolved: facebook#39008

Changelog: [Internal] - Adjust RawPropsPropNameLength's type to account for increased number of props

While investigating why we needed to back out D48288752 I discovered that the root cause was that the `items_` vector in `RawProsKeyMap` was now a size greater than 255 which becomes an issue because `items_`'s indices are statically cast to `RawPropsPropNameLength` (previously alias to `uint8_t`).

This diff updates `RawPropsPropNameLength` to be an alias to `uint16_t` so the current issue is resolved as well as adding an assert to ensure (however unlikely) that this happens again.

Differential Revision: D48331909

fbshipit-source-id: 67282a2bec83f453aa9c187b236364bf60a54290
@facebook-github-bot
Copy link
Contributor

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

vincentriemer added a commit to vincentriemer/react-native that referenced this pull request Aug 15, 2023
…of props (facebook#39008)

Summary:
Pull Request resolved: facebook#39008

Changelog: [Internal] - Adjust RawPropsPropNameLength's type to account for increased number of props

While investigating why we needed to back out D48288752 I discovered that the root cause was that the `items_` vector in `RawProsKeyMap` was now a size greater than 255 which becomes an issue because `items_`'s indices are statically cast to `RawPropsPropNameLength` (previously alias to `uint8_t`).

This diff updates `RawPropsPropNameLength` to be an alias to `uint16_t` so the current issue is resolved as well as adding an assert to ensure (however unlikely) that this happens again.

Reviewed By: rozele

Differential Revision: D48331909

fbshipit-source-id: e2942124262a95c2437561a0b9c9a5c1b7d1f1ef
vincentriemer added a commit to vincentriemer/react-native that referenced this pull request Aug 15, 2023
…of props (facebook#39008)

Summary:
Pull Request resolved: facebook#39008

Changelog: [Internal] - Adjust RawPropsPropNameLength's type to account for increased number of props

While investigating why we needed to back out D48288752 I discovered that the root cause was that the `items_` vector in `RawProsKeyMap` was now a size greater than 255 which becomes an issue because `items_`'s indices are statically cast to `RawPropsPropNameLength` (previously alias to `uint8_t`).

This diff updates `RawPropsPropNameLength` to be an alias to `uint16_t` so the current issue is resolved as well as adding an assert to ensure (however unlikely) that this happens again.

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

fbshipit-source-id: 648cae662d836915404d46198a545687744dd2b7
@facebook-github-bot
Copy link
Contributor

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

vincentriemer added a commit to vincentriemer/react-native that referenced this pull request Aug 15, 2023
…of props (facebook#39008)

Summary:
Pull Request resolved: facebook#39008

Changelog: [Internal] - Adjust RawPropsPropNameLength's type to account for increased number of props

While investigating why we needed to back out D48288752 I discovered that the root cause was that the `items_` vector in `RawProsKeyMap` was now a size greater than 255 which becomes an issue because `items_`'s indices are statically cast to `RawPropsPropNameLength` (previously alias to `uint8_t`).

This diff updates `RawPropsPropNameLength` to be an alias to `uint16_t` so the current issue is resolved as well as adding an assert to ensure (however unlikely) that this happens again.

Reviewed By: rozele

Differential Revision: D48331909

fbshipit-source-id: 5433b27d789ed0bf4475e1649041ff9f1fef1d33
@facebook-github-bot
Copy link
Contributor

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

vincentriemer added a commit to vincentriemer/react-native that referenced this pull request Aug 15, 2023
…of props (facebook#39008)

Summary:
Pull Request resolved: facebook#39008

Changelog: [Internal] - Adjust RawPropsPropNameLength's type to account for increased number of props

While investigating why we needed to back out D48288752 I discovered that the root cause was that the `items_` vector in `RawProsKeyMap` was now a size greater than 255 which becomes an issue because `items_`'s indices are statically cast to `RawPropsPropNameLength` (previously alias to `uint8_t`).

This diff updates `RawPropsPropNameLength` to be an alias to `uint16_t` so the current issue is resolved as well as adding an assert to ensure (however unlikely) that this happens again.

Reviewed By: rozele

Differential Revision: D48331909

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

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

vincentriemer added a commit to vincentriemer/react-native that referenced this pull request Aug 15, 2023
…of props (facebook#39008)

Summary:
Pull Request resolved: facebook#39008

Changelog: [Internal] - Adjust RawPropsPropNameLength's type to account for increased number of props

While investigating why we needed to back out D48288752 I discovered that the root cause was that the `items_` vector in `RawProsKeyMap` was now a size greater than 255 which becomes an issue because `items_`'s indices are statically cast to `RawPropsPropNameLength` (previously alias to `uint8_t`).

This diff updates `RawPropsPropNameLength` to be an alias to `uint16_t` so the current issue is resolved as well as adding an assert to ensure (however unlikely) that this happens again.

Reviewed By: rozele

Differential Revision: D48331909

fbshipit-source-id: 74ddbaa1bf41e58dec09794692c98dce103deb9f
@facebook-github-bot
Copy link
Contributor

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

vincentriemer added a commit to vincentriemer/react-native that referenced this pull request Aug 15, 2023
…of props (facebook#39008)

Summary:
Pull Request resolved: facebook#39008

Changelog: [Internal] - Adjust RawPropsPropNameLength's type to account for increased number of props

While investigating why we needed to back out D48288752 I discovered that the root cause was that the `items_` vector in `RawProsKeyMap` was now a size greater than 255 which becomes an issue because `items_`'s indices are statically cast to `RawPropsPropNameLength` (previously alias to `uint8_t`).

This diff updates `RawPropsPropNameLength` to be an alias to `uint16_t` so the current issue is resolved as well as adding an assert to ensure (however unlikely) that this happens again.

Reviewed By: rozele

Differential Revision: D48331909

fbshipit-source-id: 02d253761737c308e9986ac411bdcaf67d5f7f5c
@facebook-github-bot
Copy link
Contributor

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

vincentriemer added a commit to vincentriemer/react-native that referenced this pull request Aug 15, 2023
…of props (facebook#39008)

Summary:
Pull Request resolved: facebook#39008

Changelog: [Internal] - Adjust RawPropsPropNameLength's type to account for increased number of props

While investigating why we needed to back out D48288752 I discovered that the root cause was that the `items_` vector in `RawProsKeyMap` was now a size greater than 255 which becomes an issue because `items_`'s indices are statically cast to `RawPropsPropNameLength` (previously alias to `uint8_t`).

This diff updates `RawPropsPropNameLength` to be an alias to `uint16_t` so the current issue is resolved as well as adding an assert to ensure (however unlikely) that this happens again.

Reviewed By: rozele

Differential Revision: D48331909

fbshipit-source-id: 7da4c4386f855687331a4ce78b9fc230bb4ce570
@facebook-github-bot
Copy link
Contributor

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

vincentriemer added a commit to vincentriemer/react-native that referenced this pull request Aug 15, 2023
…of props (facebook#39008)

Summary:
Pull Request resolved: facebook#39008

Changelog: [Internal] - Adjust RawPropsPropNameLength's type to account for increased number of props

While investigating why we needed to back out D48288752 I discovered that the root cause was that the `items_` vector in `RawProsKeyMap` was now a size greater than 255 which becomes an issue because `items_`'s indices are statically cast to `RawPropsPropNameLength` (previously alias to `uint8_t`).

This diff updates `RawPropsPropNameLength` to be an alias to `uint16_t` so the current issue is resolved as well as adding an assert to ensure (however unlikely) that this happens again.

Reviewed By: rozele

Differential Revision: D48331909

fbshipit-source-id: 007093b50169f046bd0bcc5f2daf11c89c9cbccd
@facebook-github-bot
Copy link
Contributor

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

…of props (facebook#39008)

Summary:
Pull Request resolved: facebook#39008

Changelog: [Internal] - Adjust RawPropsPropNameLength's type to account for increased number of props

While investigating why we needed to back out D48288752 I discovered that the root cause was that the `items_` vector in `RawProsKeyMap` was now a size greater than 255 which becomes an issue because `items_`'s indices are statically cast to `RawPropsPropNameLength` (previously alias to `uint8_t`).

This diff updates `RawPropsPropNameLength` to be an alias to `uint16_t` so the current issue is resolved as well as adding an assert to ensure (however unlikely) that this happens again.

Reviewed By: rozele

Differential Revision: D48331909

fbshipit-source-id: 86a2b67bdf5090616252e9eea9165e7752793bc1
@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label Aug 15, 2023
@github-actions
Copy link

This pull request was successfully merged by @vincentriemer in 894b883.

When will my fix make it into a release? | Upcoming Releases

@facebook-github-bot
Copy link
Contributor

This pull request has been merged in 894b883.

fortmarek pushed a commit that referenced this pull request Sep 4, 2023
…of props (#39008)

Summary:
Pull Request resolved: #39008

Changelog: [Internal] - Adjust RawPropsPropNameLength's type to account for increased number of props

While investigating why we needed to back out D48288752 I discovered that the root cause was that the `items_` vector in `RawProsKeyMap` was now a size greater than 255 which becomes an issue because `items_`'s indices are statically cast to `RawPropsPropNameLength` (previously alias to `uint8_t`).

This diff updates `RawPropsPropNameLength` to be an alias to `uint16_t` so the current issue is resolved as well as adding an assert to ensure (however unlikely) that this happens again.

Reviewed By: rozele

Differential Revision: D48331909

fbshipit-source-id: f6bc3e4825f2f293d79d8cd90c40ced7cba0e3c5
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