« first day (4914 days earlier)      last day (38 days later) » 

2:21 AM
I thought type_spec of "|lll" should make all parameters optional? And it kinda does, but if I skip the first one with a named argument, it still complains?
`./php.exe -r '$d = new DateTimeImmutable; $d = $d->setDate(month: 9); echo $d->format("c");'`
>Fatal error: Uncaught ArgumentCountError: DateTimeImmutable::setDate(): Argument #1 ($year) not passed in Command line code:1
 
 
6 hours later…
8:38 AM
setDate doesn't have any optional parameters:
```
PHP_METHOD(DateTimeImmutable, setDate)
{
    zval *object, new_object;
    zend_long  y, m, d;

    object = ZEND_THIS;
    if (zend_parse_parameters(ZEND_NUM_ARGS(), "lll", &y, &m, &d) == FAILURE) {
        RETURN_THROWS();
    }
 
9:10 AM
Bro, I am adding the optional parameters
I got tired of nobody doing it so I'm doing it myself
I compiled my own version of PHP 8.4 but it won't work with named parameters for some reason. Is there something special I have to do to get it to work with named parameters, when all parameters are optional (as specified by |lll)?
Placing the bar at the head of the type type_spec has the effect of only having to specify the year, but the year still cannot be skipped by specifying a named parameter, for example month: 6, without PHP throwing the fatal error I just showed
The only downside is I don't know PHP or C, so I'm kinda stuck on small things like this
 
 
3 hours later…
12:29 PM
@QuolonelQuestions You need to edit the stub file
 
12:53 PM
Oh wow, really? I thought that was just for IDE static analysis or whatever. No idea it was actually compiled in somehow.
Also does anyone know why, when I rerun nmake, I get only the changed files compiled (as it should) but it doesn't actually update the output binary? I have to nmake clean every time, which is obviously much slower (on Windows)
People tell me incremental compilation works on Mac/Linux
Of course, it certainly doesn't notice changes to the stubs though
 
I don't know how Windows compilation works
But the stubs are used for the arginfos which holds the name of the param, the default value, etc. for Reflection and the runtime
 
Well updating it made absolutely no difference anyway
 
If the windows toolchain doesn't auto-regenerate this then run the gen_stub.php file manually
And run it with -f
Because you didn't regenerate the generated arginfos....
 
Yes I understand, let me figure out how to do it
Is it possible buildconf/configure includes running gen_stub or is it more likely I have to run it manually?
Something must have run it initially, right? Or... is the output added to VCS?
Running it, even with -f, doesn't seem to change any VCS managed files, so I don't know what it's changing (if anything)
 
The arginfo headers are added to VCS
 
1:04 PM
Oh, it does nothing if you're not in the working directory it expects... heh
Indeed, it has updated ext/date/php_date_arginfo.h now
Although, for some reason, I can't diff it because git says it's a binary file (even though it isn't)
 
There might be some weird nul byte somewhere that already exists
 
git diff -a works. I can use named arguments now, nice :)
The problem is, the semantics for optional arguments and default arguments (which applies to named arguments) are completely different xD
 
1:25 PM
How does zend_parse_parameters indicate if the parsed value was actually null?
 
You need an extra argument for this for intergers
 
What do you mean?
All the examples I can find eschew zend_parse_parameters in lieu of the more verbose ZEND_PARSE_PARAMETERS_START pattern when dealing with ?int types
 
An extra boolean pointer needs to be passed which indicates if the value was null or not
Really just use heap.space and explore the codebase to see how stuff is done
 
Thanks :)
Is it not a problem that we declare the parameters of `setDate` as long (32-bit) https://github.com/php/php-src/blob/4d7e3fcb861b7a00afeae77e5e1daa1a974378a2/ext/date/php_date.c#L3748
Yet the actual types of `php_date_obj` are defined as `timelib_sll`, which is `int64_t`?
 
1:42 PM
zend_long is 32 or 64 bit integer depending on the platform
It's not a C long, which also doesn't mean anything as that's dependent on the platform
 
Does this (partial) patch seem reasonable so far... any glaring mistakes?
https://gist.github.com/Bilge/bbb7c938b34ae06780ab1a9dfdc45d13
(it works)
 
2:03 PM
I'd need the stub diff to see if it makes sense
But it doesn't look totally stupid no
 
OK, I updated it to include the stub gist.github.com/Bilge/bbb7c938b34ae06780ab1a9dfdc45d13
"doesn't look totally stupid" is a crazy achievement, though
 
Not sure why you need the if statement tho?
As you are again checking if a value is null for the assignment
I'd just unconditionally assign dateobj = Z_PHPDATE_P(object);
 
Efficiency
 
Yeah nah I don't think so
 
In the current version, we do not parse the object into a php_date_obj, and I would not want upgrading to my version to slow down all existing scripts by doing unnecessary work
 
2:16 PM
It's just doing a pointer indirection, and guarding this behind a branch is probably gonna slow stuff more than what optimization you think it is doing
 
Oh OK
That's good feedback, thanks :)
 
 
6 hours later…
7:53 PM
@QuolonelQuestions To add to this, a branch also adds overhead; doing Z_PHPDATE_P is just pointer math and a load :-)
 
8:53 PM
Some crazy pointer math I cannot even begin to comprehend, that references calls my editor cannot even statically analyse, but sure (^:
 
 
2 hours later…
11:01 PM
@LeviMorrison Had time to play with data classes? The initial implementation is done (excluding JIT), I didn't find any unsolvable edge cases.
 

« first day (4914 days earlier)      last day (38 days later) »