Allow for arbitrary (class) attributes in stubs #8839
Conversation
|
Would it be possible to add the php-src/Zend/zend_attributes.c Lines 336 to 338 in 280b3db As that one also has a parameter, it would serve as a good test case for the all the logic. |
|
Good point, let me do this :-) |
bwoebi
force-pushed
the
arbitrary-gen-stub-attributes
branch
from
abb5725 to
a5742a8
Compare
|
@TimWolla done |
bwoebi
force-pushed
the
arbitrary-gen-stub-attributes
branch
from
a5742a8 to
40cca26
Compare
bwoebi
force-pushed
the
arbitrary-gen-stub-attributes
branch
3 times, most recently
from
fa98cbf to
74167fe
Compare
|
@kocsismate Is this good to merge? |
|
@bwoebi probably yes, but I'm on vacasion now, and can have a look until Monday. |
This can be easily extended to other types of attributes. Signed-off-by: Bob Weinand <bobwei9@hotmail.com>
Signed-off-by: Bob Weinand <bobwei9@hotmail.com>
…nce constants Signed-off-by: Bob Weinand <bobwei9@hotmail.com>
Signed-off-by: Bob Weinand <bobwei9@hotmail.com>
bwoebi
force-pushed
the
arbitrary-gen-stub-attributes
branch
from
498a602 to
2231393
Compare
|
@kocsismate Looks good now? :-) |
| @@ -131,6 +157,8 @@ class Context { | |||
| public $forceRegeneration = false; | |||
| /** @var iterable<ConstInfo> */ | |||
| public iterable $allConstInfos = []; | |||
| /** @var FileInfo[] */ | |||
| public array $parsedFiles = []; | |||
There was a problem hiding this comment.
The property type above ($allConstInfos) is a bug in gen_stub.php which is about to be fixed in a soon to be created PR of mine. The issue with it is that the gen_stub.php is part of the build system which is compatible with PHP 7.1 as of now.
build/gen_stub.php
Outdated
|
|
||
| foreach ($fileInfo->dependencies as $dependency) { | ||
| // TODO add header search path for extensions? | ||
| $prefixes = [dirname($stubFile) . "/", ""]; |
There was a problem hiding this comment.
Hmm, how does it work? The Zend/zend_attributes.stub.php is two directories deeper than ext/zend_test/test.stub.php. Wouldn't is be possible to support relative paths only, like require "../../Zend/zend_attributes.stub.php";?
There was a problem hiding this comment.
I was imagining something like include_path=.;/path/to/source/root/. At least once we add php stubs to headers, we definitely need to include such a "magic" location, whereby the source can be addressed.
I.e. in an external ext, you don't have any proper relative path to use. And the design goal here was that you can still do an in-tree build of exts without any changes. If we were to remove this in php-src, everytime you want to do in-tree development of an ext. you'd have to prepend ../../ to any include, which is not great.
bwoebi
force-pushed
the
arbitrary-gen-stub-attributes
branch
from
1b7db0a to
7b707fa
Compare
|
@kocsismate Guess all open issues are resolved now, right? |
Signed-off-by: Bob Weinand <bobwei9@hotmail.com>
bwoebi
force-pushed
the
arbitrary-gen-stub-attributes
branch
from
7b707fa to
f490f8e
Compare
|
|
||
| foreach ($fileInfo->dependencies as $dependency) { | ||
| // TODO add header search path for extensions? | ||
| $prefixes = [dirname($stubFile) . "/", dirname(__DIR__) . "/"]; |
There was a problem hiding this comment.
This is much more clear now. I barely realized that previously cwd was used for inclusion so the require wouldn't have worked if you had run gen_stub.php from a subdirectory.
There was a problem hiding this comment.
Yes, I realized that too, thus changed :-D
|
Very nice work! |
Building on top of #8776, for flexibility and usage in other extensions, i.e. avoiding hardcoding attribute declarations.
This can be easily extended to other types of attributes (params, etc.).