Port most extensions to use the new set of constant registering defines #6365
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This pull request has been imported into Phabricator, and discussion and review of the diff will take place at https://reviews.facebook.net/D48705 |
…ing defines An example PR of the changes they would allow was requested, so here is the PR, that can be merged after the main one is. Depends upon facebook#6343 (D48255)
This refactors every extension that I currently build under MSVC to use the new HHVM_RC* macros to register constants. This also removes any global constants that are the same as a define that's already provided, and also removes a few global constants that are never referenced from the C++ side, and registers the constant to a literal value instead.
Orvid
force-pushed
the
const-register-macros-changes
branch
from
eecb3ec
to
83dab04
Compare
Orvid
changed the title
…be registered twice.
Orvid
added a commit
to Orvid/hhvm
that referenced
this pull request
Oct 31, 2015
This was split out of facebook#6365 (D48705) to make reviewing easier.
Orvid
added a commit
to Orvid/hhvm
that referenced
this pull request
Oct 31, 2015
This was split out of facebook#6365 (D48705) to make reviewing easier.
Orvid
added a commit
to Orvid/hhvm
that referenced
this pull request
Oct 31, 2015
This was split out of facebook#6365 (D48705) to make reviewing easier.
Orvid
added a commit
to Orvid/hhvm
that referenced
this pull request
Oct 31, 2015
…acros This was split out of facebook#6365 (D48705) to make reviewing easier.
Orvid
added a commit
to Orvid/hhvm
that referenced
this pull request
Oct 31, 2015
This was split out of facebook#6365 (D48705) to make reviewing easier.
Orvid
added a commit
to Orvid/hhvm
that referenced
this pull request
Oct 31, 2015
…acros This was split out of facebook#6365 (D48705) to make reviewing easier.
Orvid
added a commit
to Orvid/hhvm
that referenced
this pull request
Oct 31, 2015
This was split out of facebook#6365 (D48705) to make reviewing easier.
Orvid
added a commit
to Orvid/hhvm
that referenced
this pull request
Oct 31, 2015
This was split out of facebook#6365 (D48705) to make reviewing easier.
Orvid
added a commit
to Orvid/hhvm
that referenced
this pull request
Oct 31, 2015
This was split out of facebook#6365 (D48705) to make reviewing easier.
Orvid
added a commit
to Orvid/hhvm
that referenced
this pull request
Oct 31, 2015
This was split out of facebook#6365 (D48705) to make reviewing easier.
Orvid
added a commit
to Orvid/hhvm
that referenced
this pull request
Oct 31, 2015
This was split out of facebook#6365 (D48705) to make reviewing easier. This is one of the largest changesets out of it, simply because ICU has so many constants.
Orvid
added a commit
to Orvid/hhvm
that referenced
this pull request
Oct 31, 2015
…cros This was split out of facebook#6365 (D48705) to make reviewing easier.
Orvid
added a commit
to Orvid/hhvm
that referenced
this pull request
Oct 31, 2015
This was split out of facebook#6365 (D48705) to make reviewing easier.
Orvid
added a commit
to Orvid/hhvm
that referenced
this pull request
Oct 31, 2015
This was split out of facebook#6365 (D48705) to make reviewing easier.
Orvid
added a commit
to Orvid/hhvm
that referenced
this pull request
Oct 31, 2015
This was split out of facebook#6365 (D48705) to make reviewing easier.
Orvid
added a commit
to Orvid/hhvm
that referenced
this pull request
Oct 31, 2015
…cros This was split out of facebook#6365 (D48705) to make reviewing easier.
Orvid
added a commit
to Orvid/hhvm
that referenced
this pull request
Oct 31, 2015
…acros This was split out of facebook#6365 (D48705) to make reviewing easier.
Orvid
added a commit
to Orvid/hhvm
that referenced
this pull request
Oct 31, 2015
This was split out of facebook#6365 (D48705) to make reviewing easier.
Orvid
added a commit
to Orvid/hhvm
that referenced
this pull request
Oct 31, 2015
…acros This was split out of facebook#6365 (D48705) to make reviewing easier.
Orvid
added a commit
to Orvid/hhvm
that referenced
this pull request
Oct 31, 2015
This was split out of facebook#6365 (D48705) to make reviewing easier.
Orvid
added a commit
to Orvid/hhvm
that referenced
this pull request
Oct 31, 2015
…cros This was split out of facebook#6365 (D48705) to make reviewing easier.
hhvm-bot
pushed a commit
that referenced
this pull request
Nov 4, 2015
Summary: This was split out of #6365 (D48705) to make reviewing easier. It's worth noting that `k_DISALLOWED` is both not used anywhere, and also not registered as a constant. Closes #6468 Reviewed By: sgolemon, JoelMarcey Differential Revision: D2606019 Pulled By: sgolemon fb-gh-sync-id: 005195cbaa1274a20aadb3d14556cae1af298fba
Orvid
added a commit
to Orvid/hhvm
that referenced
this pull request
Jan 25, 2016
This was split out of facebook#6365 (D48705) to make reviewing easier. It is worth pointing out that I also moved `k_INF` and `k_NAN` from `ext_std_misc` to `ext_std_math`, which is where I believe it should be, and where it also means we have a macro for registering doubles. The move of those constants has the potential to be a breaking change for some of FB's internal code. I fixed what I found in what I can build under MSVC, but there might be other places as well.
hhvm-bot
pushed a commit
that referenced
this pull request
Jul 12, 2016
Summary: This was split out of #6365 (D48705) to make reviewing easier. Reviewed By: paulbiss Differential Revision: D2870023 Pulled By: Orvid fbshipit-source-id: 861cf7c7d326ea104096b7711749cf5e76311f12
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This ports most extensions, specifically the ones that I build under MSVC and can actually test, to use the new set of macros for registering constants. This is a significant refactor, and, as part of the refactor eliminates a large number of global constants that simply weren't needed (they were the exact same, and the same name as a macro already provided, with the difference that the constant was prefixed with
k_), I'd be amazed if this didn't break at least some of FB's internal code.An example PR of the changes they would allow was requested, so here is the PR, that can be merged after the main one is.I originally thought about adding a pair ofHHVM_RC_SAME_PREF_*macros to replace theMW_CONSTmacro used by ext_imagick, but decided it's not likely to be a common pattern, so I chose not to add them.Depends upon #6343 (D48255)- Already merged