Skip to content

Fixed ZEND_BEGIN_ARG_WITH_RETURN_OBJ_INFO macro #2523

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 5 commits into from
Closed

Fixed ZEND_BEGIN_ARG_WITH_RETURN_OBJ_INFO macro #2523

wants to merge 5 commits into from

Conversation

Fleshgrinder
Copy link
Contributor

@Fleshgrinder Fleshgrinder commented May 14, 2017

The macro was expanding to a wrong macro and thus lead to compile errors upon usage. Changed it to expand to the correct macro, and added Doxygen documentation to help new users.

This is a pure internals fix, hence, I doubt that we want a real PHP bug report for this.

The macro was expanding to a wrong macro and thus leads to compile errors upon
usage. Changed it to expand to the correct macro, and added Doxygen
documentation to help new users.
Thanks Visual Studio ;(
@Fleshgrinder Fleshgrinder mentioned this pull request May 24, 2017
@nikic
Copy link
Member

nikic commented May 24, 2017

The technical change here can be easily merged, but the added documentation will require an internals discussion. We do not currently use Doxygen and as such we also do not use Doxygen-style comments anywhere in the codebase. If we want to start making use of Doxygen, this needs to be discussed first.

@Fleshgrinder
Copy link
Contributor Author

Fleshgrinder commented May 24, 2017

I can change the style to whatever is used, the thing is, there is no documentation. However, having it would greatly improve the accessibility of internals for others.

Update: Opened discussion on internals.

@krakjoe
Copy link
Member

krakjoe commented May 25, 2017

Please link to discussion for reference.

@Fleshgrinder
Copy link
Contributor Author

Internals Discussion: http://news.php.net/php.internals/99140

@remicollet
Copy link
Member

Applied in 66e5dc5
without the docblock stuff with is under discussion

@remicollet remicollet closed this Jun 30, 2017
@Fleshgrinder Fleshgrinder deleted the bugfix/arg-info-return-obj-macro branch July 1, 2017 17:50
basvanbeek added a commit to census-instrumentation/opencensus-php that referenced this pull request Jan 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants