Skip to content

Declare ext/spl constants in stubs #9226

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

Merged
merged 2 commits into from
Aug 2, 2022
Merged

Conversation

kocsismate
Copy link
Member

No description provided.

Copy link
Member

@Girgias Girgias left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with a couple of additional comments

@@ -105,6 +105,7 @@ struct _spl_filesystem_object {
#define SPL_FILE_DIR_KEY_AS_PATHNAME 0x00000000 /* make RecursiveDirectoryTree::key() return getPathname() */
#define SPL_FILE_DIR_KEY_AS_FILENAME 0x00000100 /* make RecursiveDirectoryTree::key() return getFilename() */
#define SPL_FILE_DIR_KEY_MODE_MASK 0x00000F00 /* mask RecursiveDirectoryTree::key() */
#define SPL_FILE_NEW_CURRENT_AND_KEY SPL_FILE_DIR_KEY_AS_FILENAME|SPL_FILE_DIR_CURRENT_AS_FILEINFO
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this even need to be defined? Wouldn't it be possible to compose the constant values in the stub? (or am I asking for a feature which does not yet exist)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it's not strictly required to have, but I prefer not to put too much C code into stubs, so I usually create a new macro when an expression needs to be used.

* @var int
* @cvalue SPL_FILE_NEW_CURRENT_AND_KEY
*/
public const NEW_CURRENT_AND_KEY = UNKNOWN;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public const NEW_CURRENT_AND_KEY = UNKNOWN;
public const NEW_CURRENT_AND_KEY = self::CURRENT_AS_FILEINFO|self::KEY_AS_FILENAME;

Would be the idea

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm, great idea, I'll try it if gen_stub properly support it

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, unfortunately, constant evaluation doesn't work properly :( So I'll stay with my original implementation

@kocsismate kocsismate merged commit b73f139 into php:master Aug 2, 2022
@kocsismate kocsismate deleted the const-spl branch August 2, 2022 14:37
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.

2 participants