« first day (1430 days earlier)      last day (3514 days later) » 

12:00 AM
I want it merged
 
that's cool (Tau)
 
spam the pull request with positive comments :p
Well, chip in at least
It's a bit negative just now
 
@AndreaFaulds Well... my sanity is waning: Class 'Collections\var_dump' not found in Zend/tests/return_types/015.php on line 9
 
@LeviMorrison ...what on earth cause that?
 
Only happens with return types :(
I dunno.
 
12:01 AM
@AndreaFaulds not arguing anymore, but zero is far worse than any other option
 
I've corrupted something maybe?
 
@CSᵠ with bigints there are no other options
@LeviMorrison I'll have a look at your patch
@LeviMorrison No pull request?
 
@AndreaFaulds Currently, (int)INF already yields 0 on my machine.
 
@AndreaFaulds No, I have a branch though. Link soon (gotta push up latest changes)
 
@Ja͢ck Yeah, but it doesn't do so on all machines
 
12:03 AM
Or is that by accident? :)
Ah
 
@LeviMorrison Thanks. Pull request it!
Pull requests make it easier for others to review because you can add inline comments etc.
 
The current cast of NaN to -MAXINT is certainly strange, but personally I'd rather make it null than just a 0.
 
it's all a huge mess
 
That's an oversimplification imo
 
12:06 AM
i give up and sanitize the shit out of my inputs and every step on the way as usual...
 
@Ja͢ck Explicit casts never fail, and always result in the desired type.
 
(int)INF => 0 is also stupid
 
why is it a good idea to addopt a bad decision only because it has been addopted before
 
And don't worry about things like naming or style; I haven't audited it yet. This is still "get it working" phase.
 
12:07 AM
@AndreaFaulds Exactly
 
@LeviMorrison I'll checkout your branch locally and see what happens
 
@AndreaFaulds I have a bug with opcache somewhere; maybe this is the same bug. I dunno.
 
Also, your avatar is still as adorable as ever.
 
@AndreaFaulds Yeah, I know what explicit casts should do, but a silent conversion of a computational error shouldn't go un"notice"d :)
 
@Ja͢ck Oh sure. I'd like to add some E_NOTICEs here and there... to implicit casts at least. The whole point of an explicit cast is to silence error.
@LeviMorrison Are you compiling with --enable-maintainer-zts? TSRMLS_CC errors.
 
12:13 AM
>echo (int)(new stdClass);

Notice: Object of class stdClass could not be converted to int
^^ notice on explicit cast
 
@Ja͢ck Yes, that's due to a quirk of there being no separation between explicit and implict object casts.
That notice is emitted inside the default casting handler(!)
 
@AndreaFaulds No, but it should be easy to fix because until recently I was building with it.
 
@LeviMorrison Why'd you stop?
 
Unintentional.
Fixing now.
 
@LeviMorrison I have a script that calls configure with a certain set of arguments, so I never forget.
oa-res-27-90:php-src ajf$ cat ../config
#!/bin/sh
make clean
./vcsclean
./buildconf
YACC=/usr/local/opt/bison27/bin/bison ./configure --disable-all --enable-debug --enable-maintainer-zts --enable-phpdbg "$@"
 
12:16 AM
@AndreaFaulds Still, the NaN shouldn't silently convert imo.
 
@Ja͢ck It shouldn't in implicit casts. I think it probably should in explicit casts. We should add safe casting functions, though.
 
because unlike "123abc" whereby "abc" is not important, NaN is important.
the fact that (int)[] doesn't raise a notice is also bad imo
 
@AndreaFaulds Fixed. You can pull.
 
anyway, time to bring kiddos to school, laters
 
Er, s/implicit/explicit, s/explicit/implicit
@LeviMorrison Yay, it compiles now ^^
 
12:22 AM
You should have between 2 and 4 errors depending on if you built opcache when you run make test TESTS=Zend/tests/return_types
 
@LeviMorrison Good news:
 
Oh, but it segfaulted for me this time.
 
oa-res-27-90:php-src ajf$ sapi/cli/php Zend/tests/return_types/015.php
Segmentation fault: 11
 
It wasn't segfaulting earlier.
 
Hah, you ninja'd me
Let's see what valgrind says...
 
My guess: should be doing a zend_string_addref somewhere.
 
@LeviMorrison Lots and LOTS of invalid reads
==30737== Invalid read of size 8
==30737==    at 0x1002DB8F0: zend_get_class_fetch_type (in sapi/cli/php)
==30737==    by 0x1002E8998: zend_compile_return_type (in sapi/cli/php)
This is the first
 
Good, I actually call that one.
Hmm.
 
OK
return_type->name is the problem
 
Except... in my debugger it is has a valid zend_string *
... though its gc.refcount is 16792144
 
12:31 AM
So it's not a valid zend_string :D
 
Well, the value it points to is correct, and has correct length.
 
The value has probably been freed
 
I might know why.
Gotta look through zend_resolve_class_name
 
Are you using valgrind, btw?
 
No, I don't have it on Mac.
I could get it but haven't bothered.
 
12:42 AM
brew install valgrind...?
 
I do not use Homebrew.
Install everything by hand.
 
Your loss.
@LeviMorrison Why did you create this new zend_type_decl thing exclusively for return types?
Why not do whatever parameter types do?
 
Two reasons, actually.
First, arginfo is not suitable for reuse for this purpose.
Secondly, arginfo should be using zend_string * and it is not (there is a TODO for it)
When I implement nullable types I intend to unify them.
Hm. I still cannot find where I am missing an addref.
 
1:01 AM
Aha
No, you have an extra release, possibly
One sec...
Hmm
Comment zend_string_release(class_name); in zend_compile_return_type_info and PHP will leak memory, but not segfault
 
Uhm, why?
 
Because presumably you were unnecessarily releasing that string
 
I am pretty sure it is required.
At least in other cases.
This only seems to happen with namespaced stuff.
 
zend_resolve_class_name returns a new string, no?
 
Yeah, if I don't do it PHP tells me I leaked memory and that test doesn't segfault but fails because it leaks memory.
 
1:07 AM
Why would you want to destroy it?
You want to destroy it at shutdown, presumably...
 
When an opcode is destroyed it will get freed.
I think zend_resolve_class_name must not return a copy under some circumstance.
 
@LeviMorrison No, sometimes it adds a ref
 
That makes it so hard to call :/
This is not the first time struggling with this function.
 
wat
it acts like a copy-on-write operator
That's not hard to work with
 
My point is when I call zend_resolve_class_name I have no idea if I need to release it or not.
 
1:12 AM
@LeviMorrison Huh? The answer is you never want to release it
It gives you a thing
You are keeping a pointer to it
 
No, I have to release things when they add refs.
 
Why, dear god why, would you immediately free it?
 
You don't get it.
It sometimes adds a reference.
Sometimes it doesn't.
 
No...
 
Whenever you add a reference you must release it.
 
1:12 AM
No.
 
It's a copy-on-write operation.
 
If I don't it leaks memory.
Just try it yourself.
 
I did.
Your problem is you're not cleaning this up on shutdown.
 
I am; look at Zend/zend_opcode.c
 
1:13 AM
@AndreaFaulds I threw in a little love on your tau pull request. Hope it makes sense, I've been awake for long enough to get a bit loopy.
 
@LeviMorrison Isn't it zend_execute_API.c where functions are destroyed?
Oh.
Huh, you are releasing it, weird.
 
Okay. Now you are on the same page as me.
I think.
 
Oh.
I see where the leak is
static void zend_compile_return_type_info(zend_function *func, zend_ast *ast TSRMLS_DC) /* {{{ */
{
	zend_string *class_name;
	if (ast->kind == ZEND_AST_TYPE) {
		func->common.return_type.kind = ast->attr;
		return;
	}

	class_name = zend_ast_get_str(ast);
	class_name = zend_resolve_class_name(class_name, ast->attr TSRMLS_CC);

	func->common.return_type.kind = IS_OBJECT;
	func->common.return_type.name = class_name;

//	zend_string_release(class_name);
}
/* }}} */
class_name = zend_resolve_class_name(class_name, ast->attr TSRMLS_CC);
You're getting rid of your pointer to the original name, which _resolve_class_name won't free for you
 
But it only does that sometimes!
 
Nope.
 
1:17 AM
I hate this function!
 
Fixed it.
static void zend_compile_return_type_info(zend_function *func, zend_ast *ast TSRMLS_DC) /* {{{ */
{
	zend_string *class_name, *resolved_name;
	if (ast->kind == ZEND_AST_TYPE) {
		func->common.return_type.kind = ast->attr;
		return;
	}

	class_name = zend_ast_get_str(ast);
	resolved_name = zend_resolve_class_name(class_name, ast->attr TSRMLS_CC);
	zend_string_release(class_name);

	func->common.return_type.kind = IS_OBJECT;
	func->common.return_type.name = resolved_name;
}
/* }}} */
 
Did you run the other tests?
 
It only breaks one, hmm... let me see
 
But it breaks them.
This is my whole point.
This function is retarded.
 
=====================================================================
FAILED TEST SUMMARY
---------------------------------------------------------------------
Generator return type must be Generator, Iterator or Traversable [Zend/tests/return_types/025.phpt]
=====================================================================
Seems unrelated...
 
1:19 AM
Oh, that one is broken anyway.
 
And valgrind no longer moans at me
I'm pretty sure this fixes it, and it seems like my hunch was correct. I think.
 
But this does sometimes add a ref.
Sometimes it returns the same string with a ref.
 
Yes, but that doesn't matter.
 
And other times it's a different string.
And how to call it isn't documented anywhere
 
Either way it's something needing releaing later
It's just like copy-on-write, I don't see what the big deal is
*releasing
 
1:21 AM
The big deal is that it isn't consistent.
It should always return a new string, or never.
 
@LeviMorrison Why does it need to be consistent? It deduplicates.
It follows copy-on-write.
It's not hard to work with.
 
The reason is that the function doesn't have the same side effects each time it is called.
That's bad.
That's always bad.
That is never good.
 
Well, do you want deduplication or not? What's the point of reference counting if you don't use it?
 
I'm fine with reference counting, but every time you add a ref you must release one.
And it only sometimes adds a ref
 
Nope.
 
1:23 AM
That makes it difficult to know when to free one.
 
It always adds a reference.
 
No, it doesn't.
 
No, listen
It returns something which needs delrefing later
It always adds a reference, just not always to the same string.
 
Exactly.
That's my whole bloody point.
 
I still don't see why this is a problem
 
1:24 AM
If it isn't the same string it should free the ref.
 
what?
 
Because you have to release the ref it returns, always, but sometimes it reassigns the parameter you passed in.
 
That sentence is ungrammatical, I can't parse it
 
Maybe your tolerance of WTF code is higher than mine.
 
>If it isn't the same string it should free the ref.
Why? Now it does different things.
 
1:26 AM
It is already doing different things.
 
It's following the same copy-on-write rules we follow everywhere. Is your opposition just to the concept of copy-on-write itself?
 
This code does not follow the principle of least astonishment.
First of all, it takes a parameter which it modifies, but also returns a string which is sometimes the same string.
And if it reassigns the string, it doesn't take care of the reference counting.
That violates the principle of least astonishment, clearly.
 
Er, no. It always takes care of reference counting.
 
It does not decrement the ref it reassigns.
 
It doesn't reassign anything
 
1:28 AM
It does; it reassigns the parameter.
class_name is changed and returned.
 
That's not an assignment, that's a return
You reassigned it
 
No, look at the code. It reassigns the name.
 
I don't see a double pointer anywhere
 
Line 749.
 
OK, why should it delref there?
Then it's a destructive function.
 
1:30 AM
It already destroyed hours of my time.
 
Because you kept trying to fight it.
 
Okay, even if you don't think this is an issue.
No, because I don't understand it
But as I was saying:
 
zend_resolve_class_name doesn't reassign anything. It returns a zend_string which needs releasing later.
That's it.
It's incredibly simple.
 
No, it is not
Your idea of simple is warped.
 
No, really. Listen for a second
 
1:31 AM
No, stop.
Stp.
 
If it always returned a new string, it would be exactly the same
 
I keep tryig to say something and you won't let me.
Stop
 
OK, you can speak
 
Even if you don't think this is an issue, there is absolutely no documentation on this function. At all. You must read its body which is non-trivial.
And when you are totally foreign to everything PHPNG changed, it is compounded.
This is a bad funciton.
Note that my previous code worked for all cases except namespaced strings.
 
It is poorly-documented, sure. It is not a bad function inherently.
 
1:33 AM
No, my calling code worked correctly in all cases except namespaced values.
 
Yes, because you didn't understand how reference counting worked
How is that the function's fault?
You were freeing the output, not the input
 
No, the point is that if you have to change how you call the function based on what you pass in, that is not a good design.
 
@LeviMorrison You don't have to change how you call the function. Ever.
You did something stupid with its result
You decided to free the result, rather than what you passed in
Why would you free the result? That never makes sense.
 
Sometimes the input and output are th same variable.
But sometimes not.
Sometimes adds a ref. Sometimes not.
(to the original string)
Anyway, there is no more value in discussing it.
 
Yes, sometimes it copies, sometimes it doesn't. The point is, even if zend string wasn't reference counted, your code makes no sense
 
1:37 AM
Say what you want, it tripped up Joe and Nikita too.
 
If you imagine it as always duplicating, which is what it essentially does, your code still makes no sense
 
It's a bad function.
 
It is not. It's a poorly-documented function following normal conventions.
zend_string is reference-counted.
I just... don't get this. Copy-on-write is not some obscure concept, we use it in the heart of the engine.
I'll add a comment to that function to be absolutely clear what it does.
 
Don't bother.
It will cause merge conflicts.
I'll do it.
 
How would it?
I thought you hadn't touched it?
 
1:42 AM
Non fast-forward ref.
I've touched that file a lot.
 
But that function?
 
Hey I'm not a web developer, but I need help with a simple web tool if someone is willing.
 
If you are pushing to master it will be a non-fast forward ref.
Even if the changes are fully compatible, it will be non-fast forward.
 
I need to make a page that displays someone public IPV4 address as plain text
it's for hosting game servers, so users can easily find their public ip
is there a simple code for that?
 
Tell them to search "What's my IP?"
 
1:44 AM
@LeviMorrison ...huh? Why would git push php master fail?
 
@AndreaFaulds It won't, but when I merge my branch it will fail.
Because of a non fast-forward ref.
You don't understand fast-forward references? It's at the core of basically everything git does.
Why don't you understand that, Andrea?
 
Yes, I understand that
What I don't get is how you doing it avoids a merge conflict
You still have to merge master into your branch at some point
 
And at present it doesn't conflict (or at least very recently it didnt')
 
My animosity sense is tingling! I've been lurking the convo, I think you both need a beer.
 
1:47 AM
@AndreaFaulds Then you don't understand it.
Anyway, I'm done for the night.
Need to shower and go to bed.
 
Goodnight :)
 
I have to get ready for my Switzerland trip.
 
Later @Levi
 
I also need to sleep.
 
lurking here too, between reading standards
starting to feel the whole RFC is not really thought trough at all (reg. wiki.php.net/rfc/integer_semantics)
 
1:49 AM
@CSᵠ I am looking for something like this site myexternalip.com/raw, but this site shows what ever the router defaults, like for me IPV6. I need one that always shows IPV4 but the same format. Thanks
 
@CSᵠ Oh great, what are your new objections? :/
 
@AndreaFaulds the zeroing of everything
it's awful imho
 
...what, why?
 
@Ajster1989 IPv6 / IPv4 / IPv6v4 (you prolly only got ipv6?)
 
no I have both, for that I'm sure.
 
1:53 AM
@CSᵠ Are you referring to (int) casts or are you referring to the zeros in other places
Hello?
 
@AndreaFaulds int casts of course, it just blew my mind you want to cast INF to 0
it feels so wrong, but can'ty explain enough
 
@CSᵠ Well what should it cast to?
It has to cast to something.
 
also you got me angry by saying wikipedia is enough and why read standards
 

« first day (1430 days earlier)      last day (3514 days later) »