« first day (4421 days earlier)      last day (545 days later) » 

10:48 AM
abstract method overloading when?
 
Unlikely to ever happen
 
cmb
11:08 AM
What does that even mean?
 
 
1 hour later…
12:10 PM
@cmb Yeah I know, but we got what... 3 active translations, so hopefully it isn't too annoying
 
 
1 hour later…
1:31 PM
@TimWolla question, why is Random\Randomizer::__construct(null) a thing? The default could have just been Random\Randomizer::__construct(\Random\Engine $engine = new \Random\Engine\Secure()); (yes I just realized a weird design quick now...)
 
1:50 PM
@Girgias Answer: No idea, that's part of zeriyoshi's initial implementation and apparently wasn't caught during review.
 
:(
Oh well
 
I stumbled upon that myself when writing the docs, because I also thought that the default was the Secure engine and not null.
It's not great, but not horrible, I'm fine with it.
 
2:31 PM
364 line sql upsert statement (each column name and variable assignment on it's own lie) This is either going to work gloriously or go K-Boom. Too bad this an MS shop and sql server doesn't support insert or replace....
 
3:21 PM
I thought, if only there was a SQL checker like regex has at regex101.com and lo and behold, Google turns up

https://www.eversql.com/sql-syntax-check-validator/
 
3:48 PM
sql if/else only works from stored procedures?
 
4:18 PM
@bwoebi I've been seeing Warning: uv_now(): passed UVLoop handle is already closed on shutdown of the Revolt loop. I'm guessing this is from the random destruct order where the handle object has been destroyed but something calls EventLoop::now(). Thoughts on how we could handle this?
 
4:34 PM
@Trowski yes. That issue has been annoying me since the dawn of time…
A possibility would be removing the IS_OBJ_DESTRUCTOR_CALLED GC flag from the object if new operations are attempted on already destroyed loop objects
because the UV loop memory itself is only freed when the object is also freed
I.e. ensure that dtor will be called again then when it actually reaches RC=0
I.e. do that instead of PHP_UV_IS_DTORED() checks
Sounds sensible @Trowski ?
 
5:08 PM
Alright, I give up... I wanted this done for the long holiday weekend... but wish in one hand... If anyone want to poke fun at me...
0
Q: Why does my PHP SQL Server string causes error at ON?

JimbusI'm trying to upsert a array of objects into a sql server DB with a foreach loop. If I'm barking up the wrong tree, feel free to point me in the right direct. This is the 3rd different format of this query. it tests as valid on one validator site and another chokes on the word "set" the script re...

 
 
1 hour later…
6:09 PM
@bwoebi So essentially this check is unnecessary because there's no way to pass the loop object to a function once it's actually freed?
 
@TimWolla I've started coming up with a draft for the Date Exceptions and Errors — have a look at docs.google.com/document/d/… — suggestions welcome (also from others)
 
cmb
@Derick I'm not convinced that it makes much sense to have extension specific Errors (opposed to Exceptions), since these are not supposed to be caught anyway.
 
I agree, but it's already the case like this now - and we can't really change it due to BC I think
 
cmb
ah, right (still something to consider to remove in the long run)
 
@cmb ext/random has ext-specific Errors and I think that is a good thing: I consider the Throwable class to also be part of the "human readable" error message. That's why ext/random specifically has BrokenRandomEngineError, even if that really should never happen.
 
6:18 PM
I mean, we can also just choose to break BC on this in 8.3... I doubt people catch the ERrors for corrupt timezone databases for example. But a "invalid serialisation data" one seems more useful (currently an "Error")
The DatePeriodError ones should IMO become exceptions
and the one for DateRangeError
 
Having a dedicated error class possibly also makes the error message easier to Google, because it's a very specific phrase.
 
@Trowski For the loop handle, yes. For others, no.
 
@cmb @TimWolla We're not currently in a sane situation... I don't know how it ended up growing into what we now have.
 
@Derick Do you know any good article of how to set PHPStorm + xDebug + console execution? I already have it set for debugging when request comes from browser.
 
@Derick I'd drop DateSerializationError in favor of a plain Exception. After a certain recent hotly-debated RFC: Folks really should not rely on any specific type of Exception coming out of unserialize().
That also includes catching any internal errors (e.g. for @strict-properties) and wrapping them into a plain Exception like so: github.com/php/php-src/blob/…
 
6:24 PM
@TimWolla I don't disagree, but it is current an Error - switching it to an Exception breaks BC
 
@Derick Great! Thanks man. :)
 
@Derick Yeah, ext/date is the only thing that throws an Error currently. Leave it as plain Error then, don't give folks an ext-specific Throwable for unserialize(), because that's "not a good thing".
 
that makes sense
 
@Derick I think there is a small amount of value with having all of the specific exceptions be child classes of a generic DateException (and similarly all the specific errors be children of a generic DateError class), so people can catch any exception related to Date in one catch block.
 
Regarding the remainder: I believe having a DateException and DateError as "base Exceptions", similarly to ext/random makes sense. Everything else should inherit from either of the two. If there is a value-add (either programmatically or for Googling) then a sub-Exception may be created, otherwise simply using the base-Exception is fine.
… yeah what Danack said a split second before me.
 
6:31 PM
@TimWolla jinx, you owe me a coke.
 
But does it make sense for broken serialized stuff as well? As TimWolla said... we probably shouldn't
 
@Danack it doesn't even need to be a class, it can be an interface ( extensions can declared sealed interfaces, and datetime already does it ;) )
 
It also makes it 'future safer', so that if new specific exceptions get added, the catch of DateException still happens in the right place.
@SaifEddinGmati yeah, I keep forgetting that.
 
@Derick if it's going to endup in the trace, definitely useful, if not.. not sure
 
I consider unserialize() special, because a certain RFC was declined.
 
6:33 PM
FWIW, having a base DateException/ DateError was my intention, I just badly formatted the google doc
 
@SaifEddinGmati See "choice of base Exception" here: github.com/php/php-src/pull/9220
The closest thing to a broken tzdb in ext/random would be a broken CSPRNG. I've opted to use Random\RandomException (i.e. the Exception hierarchy) for that.
 
@TimWolla that is not the case for datetime tho, as it would have specific exceptions rather than catch-all exception class
 
I'm going to have to make dinner
Drop your comments on the doc if you have them, so they don't get lost. I'll turn this into an RFC in the next few days
 
@SaifEddinGmati If/When ext/random has a need for a finer granularity of Exceptions, those would inherit from Random\RandomException. RandomException is a catch-all for every "I don't have anything better for now".
 
6:45 PM
@TimWolla here's a diagram of what i mean by adding interfaces:
@Derick ^
( powered by MS paint )
 
@SaifEddinGmati ew, why extend throwable itself?
 
Yes, I understand how interfaces for Throwables work. However I do not see a value-add here: It bloats the hierarchy for no real benefit.
Catching Error should not be done anyway, and catching a DateException to grab everything from ext/date is fine.
 
@Danack so when you have an instance of DateTimeThrowable and use it, static analysis doesn't annoy you about undefined methods ;)
@TimWolla personally, i don't see myself catching DateException, matter of fact, i don't see myself catching any exception from datetime, but when an error happens, and i look at the logs, it would be really nice to be able to identify the error, and probably it's cause just from the exception name. MalformedDateIntervalStringException tells me: somewhere in my code, i tried to create a DateInterval from a string, containing invalid data, and that is all i need.
 
Yes, and you don't need an interface for that. MalformedDateIntervalStringException extends DateException is sufficient.
In fact I just added a suggestion for "UnknownDateTimeZoneException extends DateException" to the doc.
 
@bwoebi Ah yeah, that does cover other types I see. Unfortunately that's before I'd have the opportunity to remove that flag, so I'll have to expand that macro with an option to skip that check, then add another macro like UV_PARAM_LOOP_OBJ.
 
6:58 PM
@TimWolla package exception interfaces are useful, let's say i have a service similar to sentry, that displays errors that caused your application to crash, the PHP function responsible for catching errors can identify the source of the error without diving in the stack trace simply by having a per-definded map containing base interfaces and the corresponding extensions/known packages 1/2
for an extension to have 10 exceptions with no base interfaces, that's just more work, while defining an interface is not really that complicated/expensive for extensions/packages. 2/2
 
Should that macro remove the flag then?
 
@Trowski I would remove it yes
 
@SaifEddinGmati Yes, but again: No need for interfaces. It's sufficient to have a base class that also acts as a catch-all for everything you don't have anything more specific for. Instead of using class DateException implements DateExceptionInterface, just use … class DateException.
And then you can further split the DateException by inheriting from it.
 
@TimWolla having base interfaces means you don't need base classes
 
I believe interfaces for Exceptions are only really useful, if the classes that throw those Exceptions also implement an interface.
So class TimsHttpClientException extends \Exception implements ClientExceptionInterfaceFromPsr18.
 
7:06 PM
@bwoebi The destructor will be called again, correct? I'm not familiar with how the GC works.
 
yes
 
@bwoebi Can you have a look at github.com/php/php-src/pull/9810 You're more qualified than I am to see if that's a reasonable fix for the issue with generators.
 
@Derick Bah, BC really prevents a sane Exception hierarchy for ext/date. I just realized that DateTimeImmutable::createFromFormat does not throw, but returns false for inputs that do not match the format. That would've been a prime example of where specific subclasses provide a real value-add.
I'm so happy that ext/random did not need to concern itself with that.
 
Would love a version of the ext/date factories/constructors that throw. Also on "rollover dates" (30th Feb becomes March). Could a solution be a flags parameter similar to json_*?
 
A flag parameter is never the right solution.
 

« first day (4421 days earlier)      last day (545 days later) »