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 a forward declaration of RCTMethodInfo #15788

Closed
wants to merge 1 commit into from

Conversation

iegik
Copy link

@iegik iegik commented Sep 4, 2017

declares an anonymous structure and creates a typedef for it. Thus, with this construct, it doesn't have a name in the tag namespace, only a name in the typedef namespace. This means it also cannot be forward-declared. If you want to make a forward declaration, you have to give it a name in the tag namespace.

https://stackoverflow.com/questions/612328/difference-between-struct-and-typedef-struct-in-c

(Write your motivation here.)

Test Plan

(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work. Bonus points for screenshots and videos!)

> declares an anonymous structure and creates a typedef for it. Thus, with this construct, it doesn't have a name in the tag namespace, only a name in the typedef namespace. This means it also cannot be forward-declared. If you want to make a forward declaration, you have to give it a name in the tag namespace.

https://stackoverflow.com/questions/612328/difference-between-struct-and-typedef-struct-in-c
@iegik iegik requested a review from shergin as a code owner September 4, 2017 13:33
@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. GH Review: review-needed labels Sep 4, 2017
@pull-bot
Copy link

pull-bot commented Sep 4, 2017

Attention: @shergin

Generated by 🚫 dangerJS

@hramos
Copy link
Contributor

hramos commented Sep 8, 2017

As with the others, this PR needs a test plan.

@shergin
Copy link
Contributor

shergin commented Sep 8, 2017

So, why do we need this? Any real benefits?

@iegik
Copy link
Author

iegik commented Sep 9, 2017

I have no idea. Please, read carefully Stackoverflow answer. If somebody see the difference - you can apply PR, otherwise - close it w/o merge. Thanks.

@facebook-github-bot
Copy link
Contributor

@iegik I tried to find reviewers for this pull request and wanted to ping them to take another look. However, based on the blame information for the files in this pull request I couldn't find any reviewers. This sometimes happens when the files in the pull request are new or don't exist on master anymore. Is this pull request still relevant? If yes could you please rebase? In case you know who has context on this code feel free to mention them in a comment (one person is fine). Thanks for reading and hope you will continue contributing to the project.

@stale
Copy link

stale bot commented Dec 19, 2017

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Maybe the issue has been fixed in a recent release, or perhaps it is not affecting a lot of people. If you think this issue should definitely remain open, please let us know why. Thank you for your contributions.

@stale stale bot added the Stale There has been a lack of activity on this issue and it may be closed soon. label Dec 19, 2017
@stale stale bot closed this Dec 26, 2017
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. Stale There has been a lack of activity on this issue and it may be closed soon.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants