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

Document new flags for AST_TYPE (null/false) #147

Merged
merged 1 commit into from
Jan 17, 2020

Conversation

TysonAndre
Copy link
Collaborator

No description provided.

@TysonAndre TysonAndre force-pushed the doc-AST_TYPE-flags branch 2 times, most recently from 0b70600 to e8603c4 Compare January 17, 2020 14:33
README.md Outdated
@@ -267,6 +267,8 @@ ast\flags\TYPE_DOUBLE
ast\flags\TYPE_STRING
ast\flags\TYPE_ITERABLE
ast\flags\TYPE_OBJECT
ast\flags\TYPE_NULL (php 8.0 union types)
ast\flags\TYPE_FALSE (php 8.0 union types)
Copy link
Owner

Choose a reason for hiding this comment

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

// php 8.0 union types for the sake of consistency?

@nikic nikic merged commit 0a80cfb into nikic:master Jan 17, 2020
@lisachenko
Copy link

Maybe add these flags and meta to the PHP core itself? In order to make this information useful for AST extension developers )

@TysonAndre
Copy link
Collaborator Author

Maybe add these flags and meta to the PHP core itself? In order to make this information useful for AST extension developers )

In README.md

ast\get_metadata() returns metadata about all AST node kinds. The return value is a map from AST

php > var_export(ast\get_metadata()[ast\AST_TYPE]);
ast\Metadata::__set_state(array(
   'kind' => 1,
   'name' => 'AST_TYPE',
   'flags' => 
  array (
    0 => 'ast\\flags\\TYPE_NULL',
    1 => 'ast\\flags\\TYPE_FALSE',
    2 => 'ast\\flags\\TYPE_BOOL',
    3 => 'ast\\flags\\TYPE_LONG',
    4 => 'ast\\flags\\TYPE_DOUBLE',
    5 => 'ast\\flags\\TYPE_STRING',
    6 => 'ast\\flags\\TYPE_ARRAY',
    7 => 'ast\\flags\\TYPE_OBJECT',
    8 => 'ast\\flags\\TYPE_CALLABLE',
    9 => 'ast\\flags\\TYPE_VOID',
    10 => 'ast\\flags\\TYPE_ITERABLE',
  ),
   'flagsCombinable' => false,
))

@lisachenko
Copy link

My question was not about php-ast extension ;) But about PHP core itself because this information is not available there.

@TysonAndre
Copy link
Collaborator Author

My question was not about php-ast extension ;) But about PHP core itself because this information is not available there.

There's various limitations. Right now, all of this information is implicit, and is figured out by reading Zend/zend_compile.c for the most part.

https://github.com/php/php-src/blob/0a2f6c55279c506dad65a6711567107d6aceec2d/Zend/zend_ast.c#L2074-L2111

  1. AST child nodes don't have names, they have indexes. php-ast is what chooses names to assign to them.
  2. flagsCombinable and flags of ast\Metadata are also found by reading the source right now.

zend_ast_export is the closest thing (that I can find) to ast introspection in the php-src core itself, and it's limited - it just converts an AST back to similar source code, so that C developers can debug code.

  • My guess is that there wouldn't be support for introspection APIs (e.g. fetching possible flags for a node kind) unless there was something depending on them in php-src - When someone implemented new RFCs for new syntax, they might not even know such introspection APIs needed to be updated.
  • Also, php-ast differs from the real node kind in subtle ways to maintain compatibility with ASTs generated with older php versions

If #5 was done (merge php-ast into core), there might be better APIs for php-ast and other extensions to use.

  • I don't know what the status of that is. php-ast's API seems to have stabilized (certain types of new features will require bumps to current_version in the future), so it might be a good time for that.

@TysonAndre TysonAndre deleted the doc-AST_TYPE-flags branch February 15, 2020 22:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants