« first day (2056 days earlier)      last day (2906 days later) » 

4:00 PM
so, if we treat ? as being equivalent to |null, reflection for nullables is messy, right?
does this mean all existing null-default parameters are now FooBar|null?
 
@bwoebi I don't agree with that.
 
or are they special-cased?
 
I thought we agreed to keep it all separate and leave amphp/amp as is to solve the documentation issue.
 
we didn't
we'll just have legacy.amphp.org
 
@Andrea I need to check but I believe Foo $bar = null will give Foo, not ?Foo.
This is actually a problem imo but I don't think it can be fixed without BC.
 
4:01 PM
@LeviMorrison ah. hmm
 
@bwoebi Is there any reason to merge them?
 
But BC may be small because of allowsNull().
 
I feel like allowsNull on ReflectionType was a mistake
or… maybe it wasn't. hmm :/
 
The method is fine, imo.
What's bad is what assumptions people may make because of it.
In any case I think subtype polymorphism is a better fit for this problem domain than is* methods and whatnot.
Certain methods are only applicable to certain subtypes.
It's a fairly natural decomposition.
 
@LeviMorrison definitely
 
4:04 PM
I guess the question is how far down it we go.
Do we go this route:
class ReflectionUnionType extends ReflectionCompositeType {}
class ReflectionIntersectionType extends ReflectionCompositeType {}
// or this one:
class ReflectionCompositeType extends ReflectionType {
  const KIND_UNION = 0;
  const KIND_INTERSECTION = 1;
  function getTypes(): ReflectionType[];
  function getKind(): int;
}
 
not sure that's needed
 
I dislike the constants approach
 
is (Foo&Bar)|(Baz&Qux) allowed?
 
@Andrea not with the current PR, but maybe.
 
@kelunik It makes way more sense to have everything in the Amp base namespace live in one repo. Any Amp-dependent repo can just require amphp/amp, which will still be loop-independent.
 
4:06 PM
@Andrea The intention is yes.
As Bob says the current PR does not support it.
 
if so then I'd imagine you'd have a ReflectionUnionType with two ReflectionIntersectionType children with two ReflectionClassType children each
 
@Andrea right
 
@Andrea Can you clarify "that"? Do you mean the constants or further subtyping?
 
@LeviMorrison I think I dislike ReflectionCompositeType, but I don't know why. I guess I don't see what you gain from common ancestry there
 
@Trowski What do we gain there? Apart from a single require?
 
4:08 PM
@Andrea Both unions and intersections have type lists, and I imagine surely there is some case out there that doesn't care if it's a union or intersection.
 
Breaking BC with a major version isn't possible unfortunately, unless we add the version to the package name.
 
@kelunik Having the core components all together
 
you shouldn't need constants, anyway, there's instanceof
 
@kelunik what do you mean?
 
@LeviMorrison yeah, I guess the implementation would be the same
 
4:09 PM
Okay, any nitpicks with @DaveRandom's gist?
 
anyway, I'd like it if we distinguished between ReflectionTypes that are classes and those that are builtins
 
@bwoebi We can't have amphp/observable v1 and v2 simultaneously, because they conflict as long as we don't have major versions in the package and namespace.
 
given they kinda live in their own different worlds
 
Ah, callable doesn't fit in with any of the subclasses.
 
@LeviMorrison ReflectionType should be abstract
 
4:10 PM
@Andrea It already exists and it isn't abstract, but we can probably change that detail.
 
@LeviMorrison nor array. s/ReflectionScalarType/ReflectionBuiltinType or ReflectionPrimitiveType or so
(does “primitive” extend beyond scalars? I can't remember, now)
 
@kelunik And how is that related to multiple repos?
 
@LeviMorrison yeah, we can, because it has no constructor
 
@bwoebi It would be possible to update them separately.
 
@LeviMorrison likewise, ReflectionCompositeType should be abstract
 
4:12 PM
Good nitpicks.
 
@kelunik I doubt this is a benefit
We want to have the core functionality all at a same version
 
Should the proposition formally deprecate isBuiltin?
 
hmm, I looked at wikipedia, I think the best word might be built-in type, so ReflectionBuiltinType
@LeviMorrison we might want to do that for allowsNull too…
 
I don't want projects to use versions of awaitable and observable which aren't temporally related (for example)
 
@Andrea That information is going to be there because of an optimization.
For all types.
 
4:14 PM
@LeviMorrison if we add union types, definitely. if we don't, well, maybe, given it's redundant
 
@kelunik additionally observable depends on awaitable, so there's no gain
 
@bwoebi We can release a new major for observables while keeping awaitables the same.
 
@kelunik And the benefit of that is?
 
Observables are the ones more likely to change, as awaitables are already standardized then.
 
it kinda bothers me the way we'll be handling nullables, but maybe it can't be helped
 
4:16 PM
@bwoebi Code that requires only awaitables doesn't need any update?
 
plain ReflectionType with allowsNull() === true is just about tolerable for ?int
 
@kelunik hah … as if there would be any application code only depending on awaitables
 
but for, say, int|float|null, we'd be representing the nullableness twice, no?
 
It's just the same reason why we should separate the interfaces with stronger BC guarantees in Aerys.
 
we will indirectly depend in it through libs in every single codebase
 
4:17 PM
@bwoebi Most libraries won't need observables.
@bwoebi That doesn't matter.
 
@kelunik every database one and redis do
 
and if you have ReflectionUnionType you'd get three individual ReflectionTypes from it, int, float, and null, each of which individually do not allowsNull, except for null
 
It's that every single codebase needs an update if we ever standardize observables not the way we implemented it.
 
it's messy.
 
@Wes No
 
4:19 PM
I'd rather we have, say, ReflectionNullableType (or if we have unions, ReflectionUnionType), and no allowsNull anywhere in ReflectionType
 
@kelunik And that's a good thing then
 
@bwoebi No
 
yes.
 
for Foo $foo = null we'd either infer a ReflectionNullableType, or we'd mark the nullability as part of the ReflectionParameterType, not the ReflectionType
 
@kelunik Which would require another major version of either the observables repo or Amp, so what's the difference?
 
4:20 PM
Why should that be a good thing?
 
@kelunik because a lot of repos probably rely on indirect deps
 
@Trowski As said, enough tings will just depend on awaitables.
 
In the ideal case of all repositories always specifying every single dep they directly depend on, it'd be fine
but that's not the case
 
I like the argument "every single codebase needs an update... if we standardize"
 
// ?int
ReflectionNullableType { innerType => ReflectionType { __toString => "int" }, __toString => "?int" }
// int|null
ReflectionUnionType { innerTypes => [ ReflectionType { __toString => "int" }, ReflectionType { __toString => "null" } ], __toString => "int|null" }
 
4:21 PM
@bwoebi If those require a new major of a leaked dependency (observable), they have to release a new major. That's not the issue.
 
@kelunik in that case we anyway will require nearly every codebase to be updated
because the majors then are chained
 
@bwoebi Not all the libraries that just depend on awaitables, as said.
 
I'm not really sure if this is the best place to ask, but I'm starting to learn a bit about SSL Client Certificates and using them to login on a PHP website. I was wondering on what bases the private key is generated? Most importantly: Is the private key always the same for the client or not?
 
@kelunik Very few things can just depend on the awaitable interface, they'll need to require an implementation.
 
@kelunik These are negligible
 
4:23 PM
@Trowski Yes, but an awaitable implementation, not both an observable and awaitable impl.
 
@LeviMorrison do you agree it'd be cleaner if we didn't have allowsNull?
 
@Andrea yes
 
Libraries just requiring the interface are very very minor, yes. But that's not what I'm talking about.
 
OTOH… the clean approach only maps cleanly to userland
 
@kelunik the number of (non core-amp) libraries not relying on observables indirectly are very minor too.
 
4:26 PM
hrmm
ugh, I'd like ReflectionNullableType, but if we do that we'd have to have a weird __toString for backwards compat
 
I'm asking because I wonder if the certificate can be used as a secure way to ban a visitor that missbehaves. i.e. if I revoke the certificate, can the user simply register a new account for a complete new certificate or would that simply not work because their private key remains the same?
 
@Andrea Honestly I'm not sure. It's a small detail to me.
 
unless we do some magic and don't imply NullableType but do imply allowsNull for Foo $foo = null
 
@Andrea Or… we can just define nullable to be the union of null and a type…
 
@bwoebi same problem
 
4:27 PM
why?
 
how do we handle Foo $foo = null?
 
@Andrea that's defaultValue?
 
@bwoebi the type is nullable though
so allowsNull is true
 
@Andrea right… It's equivalent to Foo|null $foo = null
 
@bwoebi but we can't actually produce that as the ReflectionType because backwards-compatibility
 
4:29 PM
@Andrea because BC we also need to retain allowsNull().
 
@bwoebi yeah… what I'm bothered by is using that as our representation of nullable types
 
user895378
@kelunik My point is that if you use php7 and you write self-documenting code you could easily generate documentation without docblocks.
 
I don't like that, as things are now, ?int would give you a ReflectionType with a __toString() of "int"
 
user895378
I'm not saying documentation is bad.
 
@Andrea yeh… don't let us touch that now.
 
user895378
4:31 PM
I'm saying that if you can't write code that allows you to generate documentation you've already lost.
 
because it's a nullable type… it should be "?int"
 
@rdlowrey and how do you document side-effects?
 
user895378
@bwoebi that's an addition, of course.
 
user895378
But you shouldn't need docblocks for anything.
 
@Andrea that and all this discussion below suggests that allowing ? T1 | T2 and not supporting T1 | T2 | null would be more sensible
 
4:32 PM
@rdlowrey you mean not for everything?
 
@nikita2206 D:
 
The only docblocks I have left in my current project are for nullable types and void return types
 
user895378
@bwoebi no, I mean you shouldn't need them for anything
 
nothing else remaining
 
user895378
They can be additive, but should not be required to generate documentation if you do code correctly.
 
4:33 PM
@Ocramius Looking forward to PHP 7.1, are we? :)
 
@NikiC yeah, waiting for Heroku to support it
they usually support it at release date
 
@Ocramius docblocks? I use comments :3
 
@rdlowrey ah, sure.
 
@Andrea the only thing people don't like about ? is that it's not clear if it's applied to T1 or T2 or the whole list of types... I guess that's something people would need to get used to
 
@Andrea a docblock is just a special type of a comment…
 
4:34 PM
function setFoo(/* ?int */ $foo) /* : void */ {
 
@Andrea no, void and nullability are actually things that are part of the phpdoc spec, so static introspection tools catch them
 
@Ocramius well, yes…
 
anyway, waiting for the real thing as well
meanwhile, I should test typed properties again
damn. Wish I had more time :-\
 
// a modest proposal:
Foo $foo = null; // ReflectionType { allowsNull = true, __toString = "Foo" }
?Foo $foo;       // ReflectionNullableType { allowsNull = true, __toString = "?Foo", innerType = ReflectionType { allowsNull = false, __toString = "Foo" } }
this would be ugly in a way, but maintains BC and would make me happy
 
ZEND_ROPE_ADD refers to interpolated strings?
 
4:40 PM
@Dereleased yes
 
btw can we have a syntax for object literals yet
for destructuring/pattern-matching, and for creation
json_encode({foo: true, bar: true});
 
@Dereleased Opcode that gives you enough rope to hang yourself. PHP is famous for it.
 
{foo: $foo, bar: $bar} = $myObject;
new FooBar { foo: $bar, bar: $bar };
good idea y/y
 
@NikiC I thought we'd rather gun our feet in PHP?
 
@bwoebi Why not both?
Doppelt hält besser
 
4:43 PM
:-D
 
I wanna have pattern-matching
but for that we first need a richer set of possible lvalues
FooBar { foo: $foo, bar: $bar } = $myObject;
 
@Andrea we use =, not : ...
 
that would set $foo to $myObject->foo, $bar to $myObject->bar, and throw an error if $myObject isn't a FooBar
 
@Andrea the type checking can still be done by instanceof…
 
@bwoebi it could be, but if we have a type check here then we can use this later for adding pattern matching
 
4:47 PM
We can add the typecheck afterwards too.
at the time we add pattern-matching
 
@Andrea This would be a parser pain.
Because we allow pointless blocks.
 
@LeviMorrison would require AST magic
 
Maybe, but it can't be handled just by AST.
 
...
 
@bwoebi Doesn't work for deconstruction.
s/getTypes()/getCompositeTypes()/ ?
I dunno, getTypes() seemed a little to general to me.
Maybe okay.
 
4:52 PM
oh yeah
 
{foo: $bar}; can be indeed parsed as stmtlist(label(foo), stmt($bar))
 
I think the Union Types RFC might benefit from mentioning that callable has special handling, that is, that it takes priority over detecting other types
This is fairly obvious if you know what callable actually is, but nonetheless
 
If we'd use {"foo" => $bar}, it'd probably not be a parser problem
 
just a detail I noticed from the patch
 
@Andrea does that matter in any way?
whether it takes priority in impl or not
the end result is the same… it either passes or not
 
4:54 PM
hmm
well, the RFC already describes the algorithm for scalar types
does the RFC explicitly state that the order types are specified in is of no particular significance?
 
@Andrea It mainly describes the conversions.
And it also says that exact (i.e. without cast) matches are priority
(and callable, if matching, is obviously an exact match)
 
callable is a loose concept
 
callable is everything you can put as $foo(); without error on call
 
sort of.
I mean I infer that if I do callable|string|array then… it'd allow any string or array, right?
 
@NikiC I always find enough rope to shoot myself with
 
4:58 PM
@Andrea and objects with __invoke method, right
 
@NikiC actually can I borrow you for a few minutes again in the usual place?
 
@bwoebi oh right not just Closure? I hadn't realised that
 
@Andrea Closure has an __invoke method, thus it passes.
 
right…
 
Every callable object is guaranteed to have an __invoke method and vice-versa
 
5:01 PM
@Dereleased The usual place seems to be frozen ^^
 
> If double passed, cast to (if allowed, in that order) long if exact match or string not allowed, string, boolean
Wait, what
 
@NikiC Yeah, just noticed that
I guess a new usual place
 
@Dereleased That's this room.
 
so if you have long|string and pass in 1.5 you get "1.5"
that's…
 
class ReflectionClassType extends ReflectionType
 
5:03 PM
@Dereleased We're typically discussing all internals related things here since the old place froze…
 
So… how does enum types affect this?
 
I mean, it makes sense from the perspective of preserving information, but probably doesn't match intent…
 
For instance if you have a type which does not yet exist what ReflectionType do you return?
I'm not sure if unions will pass but I expect we'll one day have enums of some sort. It really needs to be considered.
 
@LeviMorrison I mean, Class is a bit shaky anyway given interfaces, but we don't have a word for “thing in the namespace that classes, interfaces and traits inhabit”
 
Sure, but that's a class-like type misnomer we're already aware of.
Enums are definitely a different type of thing.
 
5:05 PM
@bwoebi I'm not discussing internals from a perspective that's useful. Not right now, at least. Right now I'm just trying to learn a bunch of stuff.
 
@LeviMorrison I would be okay with considering enums to be “classes”, but things like typedefs are messier
 
Look at the method list for ReflectionClass; an enum would not need most of those things.
It's further indication to me they should be separate.
 
@Dereleased I've just found your old room … well… that type of discussion is still not unwelcome here ^^
 
Well, either way. Trying not to clutter things
 
hi guys
 
Anonymous
5:16 PM
Yo
 
@bwoebi @Andrea We can't provide a ReflectionClassType.
For types that don't exist we still need a ReflectionType that does not throw an exception.
Or rather we can provide a ReflectionClassType only when the type already exists.
 
@LeviMorrison or we can just trigger an autoload when the ReflectionClass shall be fetched
 
guys, what caching system do you recommend?
for php obviously :D
 
@bwoebi We would need to be careful to not throw an exception if it does not exist.
 
@LeviMorrison In case we ask for the reflection class, it should return null
 
5:20 PM
That is consistent with the current designs which throw exceptions.
Rather we need to just represent it such that it works fine.
 
in the VM, regardless of what the prefix is when an op_array is dumped, there is just one set of incrementing numbers for non-CV, right?
What is the idiomatic way to get that number?
 
@Dereleased op_array->T you mean?
 
L1-7 {main}() /opt/files/parser/tests/test_opt/5.php - 0x10747a0c0 + 6 ops
L3 #0 ASSIGN $i 0
L5 #1 ECHO $i
L6 #2 PRE_INC $i
L7 #3 IS_SMALLER $i 10 ~2
L7 #4 JMPNZ ~2 J1
L7 #5 RETURN 1
the 2 in ~2, for example
opline->???
well, I assume there is a macro for it, actually
 
ah, sec
@Dereleased EX_VAR_TO_NUM(opline->var) - op_array->last_var
 
perfect
 
5:27 PM
@Dereleased EX_VAR_TO_NUM() gives you the offset from the first CV (in zval slots, i.e. execute_data + sizeof(execute_data) + 16 * offset == location where the temporary will be), then substract the offset of the last cv
 
I confirmed that the __toString on implicit nullable types because of a default value of null will only provide the class name, but allowsNull() is true.
/cc @Andrea @bwoebi
I believe is the same for explicit nullable types as well.
It would be a BC break to change it for implicit ones, but we may be able to discriminate it for userland types.
I'll just have to play around.
 
Mostly makes sense to me, but I'm really bad at C, so one bit of clarity. I'm gonna try to explain what I understand, you tell me where the gap is:

execute_data // ptr to exec data
+ sizeof( execute_data ) // next mem slot after ptr to exec data
+ 16 * offset // offset is just an incremental list for temp vars, multiplied by 16 because that's the size of the pointer to each one?
- op_array->last_var // I have guesses, but they all seem dumb
 
@Dereleased 16 is the size of the zvals
@Dereleased last_var is just the number of cvs (i.e. the index of the last variable)
 
@bwoebi I'm trying to imagine the memory segment in my head here, I think I've got it making sense
so I'm backing up into the segment where all the variables are, which is technically inside execute_data ?
Maybe I don't need to understand this one right now, we'll come back to it
I have a pretty close mental approximation at the moment
 
@Ocramius Do you use Reflection much?
 
5:41 PM
@bwoebi so the CVs are just 1 wide?
 
@Dereleased no, they are 16 bytes too
@Dereleased next to execute_data is more precise.
 
@LeviMorrison yarp
in tools tho, not core domain ofc :P
 
Okay, I'm going to ask you for some feedback on some Reflection changes in a bit then.
 
@Ocramius wait… wat … you're not using reflection to add numbers in your main code??
 
@bwoebi What if I want to get the "number" of a CV?
 
5:47 PM
@Dereleased Then just EX_VAR_TO_NUM(opline->var)
 
so CV, TMP, all the same?
 
yes, they're all allocated next to each other on the VM stack
 
@LeviMorrison cool
 
Naming question: if I were to make a variant of RETURN_STRINGL that works with a string literal and uses sizeof() - 1 so that there is only 1 parameter, what should I name that macro?
 
@LeviMorrison you may cc @asgrim about it. He's been pretty much mirroring the reflection API in userland, and knows a lot of quirks too
 
5:51 PM
@Andrea btw. union types "rules" are really just if type == x, then try to cast to the available types in a defined order.
 
Actually, is that RETVAL_STRING?
 
@bwoebi Perhaps you want to update the RFC with some of the language you used in your reply on internals.
 
We just need a separate rule for every type (obviously) … but it's nothing else than what we do for weak casts too @Andrea
@Trowski May you please propose a good wording for that please?
 
Ah, we cannot distinguish between explicit and implicit nullable types in Reflection with the current code.
 
@LeviMorrison anyway
 

« first day (2056 days earlier)      last day (2906 days later) »