Hello!
I'm converting my code to use short closures where possible, and I ran into
a problem using compact()
.
Basically, the names used in compact()
cannot be accessed due to a bug,
reported in 2019 still in PHP 7.4 (ID 78970).
https://bugs.php.net/bug.php?id=78970
It seems to me to be a reasonable problem and one that needs attention, as
the message is not that "compact cannot be used here", but that "the
variable does not exist".
The code below may reproduce the problem:
$x = 123;
(fn() => compact('x'))();
Is there any possibility of this being fixed? I would love to help, but I
don't have much C programming skills, unfortunately.
Atenciosamente,
David Rodrigues
Hello!
I'm converting my code to use short closures where possible, and I ran into
a problem usingcompact()
.Basically, the names used in
compact()
cannot be accessed due to a bug,
reported in 2019 still in PHP 7.4 (ID 78970).https://bugs.php.net/bug.php?id=78970
It seems to me to be a reasonable problem and one that needs attention, as
the message is not that "compact cannot be used here", but that "the
variable does not exist".The code below may reproduce the problem:
$x = 123;
(fn() => compact('x'))();Is there any possibility of this being fixed? I would love to help, but I
don't have much C programming skills, unfortunately.
I'd rather hope for compact()
to finally be deprecated and targeted for
removal 😛
The fact that it still exists precludes (or at least complicates) future
optimization of scope + inlining in the engine.
Similar thoughts towards extract()
, I'd say.
I'd rather hope for
compact()
to finally be deprecated and targeted for
removal 😛
I think compact()
is a good function for transferring variables from one
point to another, but I would think about making improvements as it is
confusing (uses the variable name, rather than the variable itself).
Regarding the bug, if we use an array it should work perfectly:
$x = 123;
(fn() => [ 'x' => $x ])();
Would it be possible to automatically convert compact()
to an array at
runtime? So I imagine that any necessary optimization can take place
directly over the resulting array, rather than the compact itself.
Atenciosamente,
David Rodrigues
Em qua., 19 de out. de 2022 às 14:09, Marco Pivetta ocramius@gmail.com
escreveu:
On Wed, 19 Oct 2022, 19:04 David Rodrigues, david.proweb@gmail.com
wrote:Hello!
I'm converting my code to use short closures where possible, and I ran
into
a problem usingcompact()
.Basically, the names used in
compact()
cannot be accessed due to a bug,
reported in 2019 still in PHP 7.4 (ID 78970).https://bugs.php.net/bug.php?id=78970
It seems to me to be a reasonable problem and one that needs attention, as
the message is not that "compact cannot be used here", but that "the
variable does not exist".The code below may reproduce the problem:
$x = 123;
(fn() => compact('x'))();Is there any possibility of this being fixed? I would love to help, but I
don't have much C programming skills, unfortunately.I'd rather hope for
compact()
to finally be deprecated and targeted for
removal 😛The fact that it still exists precludes (or at least complicates) future
optimization of scope + inlining in the engine.Similar thoughts towards
extract()
, I'd say.
śr., 19 paź 2022 o 19:18 David Rodrigues david.proweb@gmail.com
napisał(a):
I'd rather hope for
compact()
to finally be deprecated and targeted for
removal 😛I think
compact()
is a good function for transferring variables from one
point to another, but I would think about making improvements as it is
confusing (uses the variable name, rather than the variable itself).Regarding the bug, if we use an array it should work perfectly:
$x = 123;
(fn() => [ 'x' => $x ])();Would it be possible to automatically convert
compact()
to an array at
runtime? So I imagine that any necessary optimization can take place
directly over the resulting array, rather than the compact itself.
How would you like it to work, if you pass a variable name variable to
compact then?
$x = 123;
$name = 'x';
(fn () => compact($name))();
I agree with the idea of deprecating compact()
& extract()
and in
long-term variable of variables as well.
Cheers,
Michał Marcin Brzuchalski
How would you like it to work, if you pass a variable name variable to
compact then?
$x = 123;
$name = 'x';
(fn () => compact($name))();
Although it is difficult to make it work in general (of course), there is the specific case of names given as literal strings, as in the example provided by the OP, that does not suffer from the impossibility of static analysis.
I agree with the idea of deprecating
compact()
&extract()
and in
long-term variable of variables as well.
However, in compact('x', 'y')
, the name of the variables are not variable, they are constant.
—Claude
It seems to me to be a reasonable problem and one that needs attention, as
the message is not that "compact cannot be used here", but that "the
variable does not exist".
I'd just like to point out that the error message here is 100% correct:
there is nothing wrong with using compact()
inside a closure, it just
only has access at run-time to the variables which were captured when
the closure was created, which are determined at compile-time.
$foo = fn() => [$a, compact('a', 'b')];
is essentially compiled to:
$foo = function (use $a) { return [$a, compact('a', 'b')] };
When you later execute $foo, compact()
can see the local $a, but there
is no $b anywhere in the compiled definition.
Although it is difficult to make it work in general (of course), there is the specific case of names given as literal strings, as in the example provided by the OP, that does not suffer from the impossibility of static analysis.
While it would be possible for the compiler to special-case this
scenario, I think it could be rather confusing to have a function that
is sometimes evaluated at compile-time and sometimes at run-time (other
than as a transparent performance optimisation). For instance, each of
the following can be fully evaluated to a constant at compile-time,
but which ones would be?
compact('a' . 'b');
$name = 'a';
compact($name);
$x = 1;
compact('a' . $x);
$x = 1;
$x++;
compact('a' . $x);
I think it would be better to look at a new syntax for specifying such
arrays that doesn't rely on a run-time function, similar to how in JS {
foo, bar } is short-hand for { foo: $foo, bar: $bar }
This has come up before, for instance in Nikita's discussion of named
parameter syntaxes:
https://wiki.php.net/rfc/named_params#shorthand_syntax_for_matching_parameter_and_variable_name
As well as removing the need to pass a variable name as a string, this
would allow combining explicit and automatic names in one literal, e.g.:
// Current array literal
$x = [ 'foo' => $foo, 'bar' => someFunc() ];
// compact()
$bar = someFunc();
$x = compact('foo', 'bar');
// Shortest variation in Nikita's RFC linked above
$x = [ :$foo, bar: someFunc() ];
I'm personally not a fan of this coding style, because I think variables
should be named to be meaningful in the current scope, not somewhere
they're coming from or going to, but a dedicated syntax would at least
allow that flexibility.
Regards,
--
Rowan Tommins
[IMSoP]
$foo = fn() => [$a, compact('a', 'b')];
is essentially compiled to:
$foo = function (use $a) { return [$a, compact('a', 'b')] };
Regarding this, in case you're literally saying that internally "fn()"
turns into a common "function", and getting back to the main topic, would
it be possible to do the same with compact()
?
fn() => compact('dummy')
turns into:
fn() => [ 'dummy' => $dummy ]
which turns into:
function() use($dummy) { return [ 'dummy' => $dummy ]; }
Atenciosamente,
David Rodrigues
Regarding this, in case you're literally saying that internally "fn()"
turns into a common "function", and getting back to the main topic,
would it be possible to do the same withcompact()
?fn() => compact('dummy')
turns into:
fn() => [ 'dummy' => $dummy ]
which turns into:
function() use($dummy) { return [ 'dummy' => $dummy ]; }
Yes, that is what I meant by "it would be possible for the compiler to
special-case this scenario"; I explained why I think that would be a bad
idea, and suggested an alternative.
Regards,
--
Rowan Tommins
[IMSoP]
Regarding this, in case you're literally saying that internally "fn()"
turns into a common "function", and getting back to the main topic,
would it be possible to do the same withcompact()
?fn() => compact('dummy')
turns into:
fn() => [ 'dummy' => $dummy ]
which turns into:
function() use($dummy) { return [ 'dummy' => $dummy ]; }Yes, that is what I meant by "it would be possible for the compiler to
special-case this scenario";
Well, in the general case, the compiler cannot special-case this,
because compact()
is a normal function, so might be used as "variable
function", or may not refer to the top-level compact()
at all (the
called symbol may not be fully qualified).
I explained why I think that would be a bad
idea, and suggested an alternative.
ACK
--
Christoph M. Becker