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

Fix New Arch build failing with -std=c++20 #42138

Merged
merged 1 commit into from
Jan 6, 2024

Conversation

tido64
Copy link
Collaborator

@tido64 tido64 commented Jan 4, 2024

Summary:

React-Fabric fails to build because implicit constructors are no longer generated.

❌ /~/example/ios/Pods/Headers/Public/React-Fabric/react/renderer/core/RawPropsParser.h:50:24: no matching constructor for initialization of 'PropsParserContext'
    PropsParserContext parserContext{-1, contextContainer};
                       ^            ~~~~~~~~~~~~~~~~~~~~~~

This was fixed in 0.73 here: e1876af#diff-696b8d801910a87b0ba34645cfefcf01238c3f670d0e956a274091a6c950cc12

Changelog:

[IOS] [FIXED] - React-Fabric fails to build with -std=c++20 because implicit constructors are no longer generated

Test Plan:

n/a

@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: Microsoft Partner: Microsoft Partner labels Jan 4, 2024
@tido64
Copy link
Collaborator Author

tido64 commented Jan 4, 2024

Closed by accident.

@tido64 tido64 reopened this Jan 4, 2024
@NickGerleman
Copy link
Contributor

NickGerleman commented Jan 4, 2024

I suspect this is a C++ 20 thing instead of an XCode 15 thing. At least that bit of code is known to have a compat issue there. But older versions of RN shouldn’t be using that.

@tido64
Copy link
Collaborator Author

tido64 commented Jan 5, 2024

I suspect this is a C++ 20 thing instead of an XCode 15 thing. At least that bit of code is known to have a compat issue there. But older versions of RN shouldn’t be using that.

I think you're right. Still, there will be cases where the hosting app itself may be using C++20 and including some headers from react-native. This patch doesn't break anything while allowing this usage scenario.

@NickGerleman
Copy link
Contributor

I suspect this is a C++ 20 thing instead of an XCode 15 thing. At least that bit of code is known to have a compat issue there. But older versions of RN shouldn’t be using that.

I think you're right. Still, there will be cases where the hosting app itself may be using C++20 and including some headers from react-native. This patch doesn't break anything while allowing this usage scenario.

Yeah, the underlying fix is good. It might be worth updating commit title/message/changelog to be about fixing C++ 20 compat, instead of XCode 15, for posterity/recordkeeping.

@tido64 tido64 changed the title Fix New Arch build failure on Xcode 15 Fix New Arch build failing with -std=c++20 Jan 5, 2024
@NickGerleman
Copy link
Contributor

Looks like there are some unrelated failures, but otherwise, it seems like we have consensus on merging this.

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. p: Microsoft Partner: Microsoft Partner Pick Request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants