Mailing List Archive

Top-posting
For those who want to know exactly what's going on and don't want to wade through the diff or tests:

$ mkdir lib/{A,B,C}pack.pm

And in each of those, add this:

package Apack;use Bpack;1;
package Bpack;use Cpack;1;
package Cpack;my $i = 0;
while ( my ( $package, $file, $line ) = caller( $i++ ) ) {    push @Cpack::callers, "$file:$line";}1;

And then create run.pl:

#!/usr/bin/env perl
use lib 'lib';use Apack;use Data::Dumper;print Dumper( \@Cpack::callers );

If you have a patched Perl, you get this:

$VAR1 = [.          'lib/Bpack.pm:2',          'lib/Bpack.pm:2',          'lib/Bpack.pm:2',          'lib/Apack.pm:2',          'lib/Apack.pm:2',          'lib/Apack.pm:2',          'run.pl:4',          'run.pl:4',          'run.pl:4'        ];

That's perfectly sane and you can figure out what's going on (though I confess I don't know why some stack frames are repeated).
For an unpatched Perl, you get something like this:

$VAR1 = [.          'lib/Bpack.pm:2',          'lib/Cpack.pm:0',          'lib/Cpack.pm:0',          'lib/Apack.pm:2',          'lib/Cpack.pm:0',          'lib/Cpack.pm:0',          'run.pl:4',          'lib/Cpack.pm:0',          'lib/Cpack.pm:0'        ];

I cannot use that stack trace for debugging.
I have verified the buggy behavior all the way back to 5.8.9.
The patch which reverted the fix notes the following:

commit c0d05305c156c83e4f9f3a207451b3175fbb7f24 (HEAD)Merge: c6b2e294d8 79f75eaa71Author: David Mitchell <davem@iabyn.com>Date:   Mon Apr 27 22:04:23 2020 +0100
    [MERGE] Revert BEGIN { caller() } fixups
    These commits were intended to fix a problem with stack backtraces    reporting wrong file and line numbers in nested use's.
    A side-effect of the commits was to fix the package name returned by    caller() too; but quite a few distributions were relying on the old    behaviour.
    So for now, revert to the old behaviour and re-address after 5.32.0 is    released.
    The reverted commits are:
    v5.31.6-141-gf2f32cd638 avoid identical stack traces    v5.31.9-122-gee428a211d docs: clarify effect of $^H, %^H, ${^WARNING_BITS}    v5.31.9-162-gad89278aa2 fixup to "avoid identical stack traces" - try 2

I don't know which distributions were failing or why, but having a few distributions making it harder for all Perl developers to debug their programs seems backwards.
What can we do to get this fixed?
Best,Ovid-- IT consulting, training, specializing in Perl, databases, and agile development
http://www.allaroundtheworld.fr/. 

Buy my book! - http://bit.ly/beginning_perl






On Thursday, 13 January 2022, 06:05:07 CET, demerphq <demerphq@gmail.com> wrote:





This subject has come up multiple times and is a source of deep frustration for me, and deep confusion for the many non-perl developers who work on perl code at Booking.com.

This was fixed by Dave Mitchell in f2f32cd638746f538da6db804dab6dd54e654f30
https://github.com/Perl/perl5/commit/f2f32cd638746f538da6db804dab6dd54e654f30

This patch was then reverted in merge commit c0d05305c156c83e4f9f3a207451b3175fbb7f24.

Thus stack traces are *still* broken. I am looking at many examples right now and it is impossible to debug what is happening. I cannot post the work examples but the patch referenced includes examples from its tests.

Can we please fix this once and for all? We are running patched perl's at work so that we can fix this (without any sign of any related fallout!), and every time we release a new perl we have to reapply f2f32cd638746f538da6db804dab6dd54e654f30. When the people who build new perls forget then we end up with mass confusion amongst our devs.

Perl has a really bad name amongst the devs we hire, this one of the reasons. Can we please please please FINALLY DO SOMETHING about this? Perl has so many weird ways to establish a dependency and this bug makes it almost impossible to debug non-trivial cases even for an expert class perl dev like myself. 

I beg of you, do not let another perl get released with this bug in place!

Thanks,
Yves



--
perl -Mre=debug -e "/just|another|perl|hacker/"

On Thu, 13 Jan 2022, 18:45 Ovid, <curtis_ovid_poe@yahoo.com> wrote:

> Top-posting
>
> For those who want to know exactly what's going on and don't want to wade
> through the diff or tests:
>
> $ mkdir lib/{A,B,C}pack.pm
>
>
> And in each of those, add this:
>
> package Apack;
> use Bpack;
> 1;
>
> package Bpack;
> use Cpack;
> 1;
>
> package Cpack;
> my $i = 0;
>
> while ( my ( $package, $file, $line ) = caller( $i++ ) ) {
> push @Cpack::callers, "$file:$line";
> }
> 1;
>
>
> And then create run.pl:
>
> #!/usr/bin/env perl
>
> use lib 'lib';
> use Apack;
> use Data::Dumper;
> print Dumper( \@Cpack::callers );
>
>
> If you have a patched Perl, you get this:
>
> $VAR1 = [.
> 'lib/Bpack.pm:2',
> 'lib/Bpack.pm:2',
> 'lib/Bpack.pm:2',
> 'lib/Apack.pm:2',
> 'lib/Apack.pm:2',
> 'lib/Apack.pm:2',
> 'run.pl:4',
> 'run.pl:4',
> 'run.pl:4'
> ];
>
>
> That's perfectly sane and you can figure out what's going on (though I
> confess I don't know why some stack frames are repeated).
>

The repeats come from the expansion of use into BEGIN, require, and the use
itself I believe.


> For an unpatched Perl, you get something like this:
>
> $VAR1 = [.
> 'lib/Bpack.pm:2',
> 'lib/Cpack.pm:0',
> 'lib/Cpack.pm:0',
> 'lib/Apack.pm:2',
> 'lib/Cpack.pm:0',
> 'lib/Cpack.pm:0',
> 'run.pl:4',
> 'lib/Cpack.pm:0',
> 'lib/Cpack.pm:0'
> ];
>
>
> I cannot use that stack trace for debugging.
>
> I have verified the buggy behavior all the way back to 5.8.9.
>
> The patch which reverted the fix notes the following:
>
> commit c0d05305c156c83e4f9f3a207451b3175fbb7f24 (HEAD)
> Merge: c6b2e294d8 79f75eaa71
> Author: David Mitchell <davem@iabyn.com>
> Date: Mon Apr 27 22:04:23 2020 +0100
>
> [MERGE] Revert BEGIN { caller() } fixups
>
> These commits were intended to fix a problem with stack backtraces
> reporting wrong file and line numbers in nested use's.
>
> A side-effect of the commits was to fix the package name returned by
> caller() too; but quite a few distributions were relying on the old
> behaviour.
>
> So for now, revert to the old behaviour and re-address after 5.32.0 is
> released.
>
> The reverted commits are:
>
> v5.31.6-141-gf2f32cd638 avoid identical stack traces
> v5.31.9-122-gee428a211d docs: clarify effect of $^H, %^H,
> ${^WARNING_BITS}
> v5.31.9-162-gad89278aa2 fixup to "avoid identical stack traces" - try 2
>
>
> I don't know which distributions were failing or why, but having a few
> distributions making it harder for *all* Perl developers to debug their
> programs seems backwards.
>

Hearty agreement.


> What can we do to get this fixed?
>

Yves

>
>

For other's reference - it broke DBIX::Class
https://github.com/Perl/perl5/issues/17663, and you know it. It's
quite late in a release cycle to bring this up, since it'd require a
lot of efforts to release a fixed version of it to CPAN.

Best regards,
Sergey Aleynikov

On Thu, 13 Jan 2022 at 14:26, Sergey Aleynikov <sergey.aleynikov@gmail.com>
wrote:

> For other's reference - it broke DBIX::Class
> https://github.com/Perl/perl5/issues/17663, and you know it. It's
> quite late in a release cycle to bring this up, since it'd require a
> lot of efforts to release a fixed version of it to CPAN.
>

Yeah, I get your point, so maybe we have to suffer one more release, but
bottom line if DBIx::Class depends on this bug then it is depending on
clearly *broken* behaviour, and you have to wonder what insanity is being
hidden.

cheers,
Yves

On 2022-01-13 6:25 a.m., demerphq wrote:
> On Thu, 13 Jan 2022 at 14:26, Sergey Aleynikov wrote:
> For other's reference - it broke DBIX::Class
> https://github.com/Perl/perl5/issues/17663, and you know it. It's
> quite late in a release cycle to bring this up, since it'd require a
> lot of efforts to release a fixed version of it to CPAN.
>
> Yeah, I get your point, so maybe we have to suffer one more release, but bottom
> line if DBIx::Class depends on this bug then it is depending on clearly *broken*
> behaviour, and you have to wonder what insanity is being hidden.

I feel that Perl 5.36.0 SHOULD fix its own broken behavior regarding stack traces.

Just don't fix it in any 5.34.x or 5.32.x etc.

The 5.36.0 release is a major release and is far enough away that DBIx::Class
should be able to get fixed to not rely on the broken behavior in time for its
release.

The fix shouldn't be put off another year. If anyone depends on something that
depends on the broken behavior, they should not upgrade past 5.34.x until their
other dependencies are updated for compatibility.

-- Darren Duncan

On Fri, 14 Jan 2022 at 11:57, Darren Duncan <darren@darrenduncan.net> wrote:

> On 2022-01-13 6:25 a.m., demerphq wrote:
> > On Thu, 13 Jan 2022 at 14:26, Sergey Aleynikov wrote:
> > For other's reference - it broke DBIX::Class
> > https://github.com/Perl/perl5/issues/17663, and you know it. It's
> > quite late in a release cycle to bring this up, since it'd require a
> > lot of efforts to release a fixed version of it to CPAN.
> >
> > Yeah, I get your point, so maybe we have to suffer one more release, but
> bottom
> > line if DBIx::Class depends on this bug then it is depending on clearly
> *broken*
> > behaviour, and you have to wonder what insanity is being hidden.
>
> I feel that Perl 5.36.0 SHOULD fix its own broken behavior regarding stack
> traces.
>
> Just don't fix it in any 5.34.x or 5.32.x etc.
>
> The 5.36.0 release is a major release and is far enough away that
> DBIx::Class
> should be able to get fixed to not rely on the broken behavior in time for
> its
> release.
>
> The fix shouldn't be put off another year. If anyone depends on something
> that
> depends on the broken behavior, they should not upgrade past 5.34.x until
> their
> other dependencies are updated for compatibility.
>

I can live with that.

I am not really convinced the patch really does break anything however, we
have been running with the fix backported for years and we never noticed
any issues with our DBIx class code. *shrug*

Yves




--
perl -Mre=debug -e "/just|another|perl|hacker/"

On 2022-01-14 3:40 a.m., demerphq wrote:
> On Fri, 14 Jan 2022 at 11:57, Darren Duncan wrote:
> I feel that Perl 5.36.0 SHOULD fix its own broken behavior regarding stack
> traces.
>
> The 5.36.0 release is a major release and is far enough away that DBIx::Class
> should be able to get fixed to not rely on the broken behavior in time for its
> release.
>
> The fix shouldn't be put off another year.  If anyone depends on something that
> depends on the broken behavior, they should not upgrade past 5.34.x until their
> other dependencies are updated for compatibility.
>
> I can live with that.
>
> I am not really convinced the patch really does break anything however, we have
> been running with the fix backported for years and we never noticed any issues
> with our DBIx class code. *shrug*

That's all the more reason to re-apply the bug fix to blead right away.

Your experience would imply that any reliance in DBIx::Class on the broken
behavior isn't in its core functionality and more in some non-core functionality
or alternately just a test is broken and main code isn't.

-- Darren Duncan

On Fri, 14 Jan 2022, 20:00 Darren Duncan, <darren@darrenduncan.net> wrote:

> On 2022-01-14 3:40 a.m., demerphq wrote:
> > On Fri, 14 Jan 2022 at 11:57, Darren Duncan wrote:
> > I feel that Perl 5.36.0 SHOULD fix its own broken behavior regarding
> stack
> > traces.
> >
> > The 5.36.0 release is a major release and is far enough away that
> DBIx::Class
> > should be able to get fixed to not rely on the broken behavior in
> time for its
> > release.
> >
> > The fix shouldn't be put off another year. If anyone depends on
> something that
> > depends on the broken behavior, they should not upgrade past 5.34.x
> until their
> > other dependencies are updated for compatibility.
> >
> > I can live with that.
> >
> > I am not really convinced the patch really does break anything however,
> we have
> > been running with the fix backported for years and we never noticed any
> issues
> > with our DBIx class code. *shrug*
>
> That's all the more reason to re-apply the bug fix to blead right away.
>
> Your experience would imply that any reliance in DBIx::Class on the broken
> behavior isn't in its core functionality and more in some non-core
> functionality
> or alternately just a test is broken and main code isn't.
>

That would be my intuition, but I don't know that we use enough DBIx to be
certain. But it's extremely hard for me to understand how such broken data
could ever be truly useful.

Yves

>
>

As per https://github.com/Perl/perl5/issues/17663#issuecomment-602244332,
the "bug" in DBIC is an issue only in a test, not in actually code.

On Fri, Jan 14, 2022 at 2:14 PM demerphq <demerphq@gmail.com> wrote:

>
>
> On Fri, 14 Jan 2022, 20:00 Darren Duncan, <darren@darrenduncan.net> wrote:
>
>> On 2022-01-14 3:40 a.m., demerphq wrote:
>> > On Fri, 14 Jan 2022 at 11:57, Darren Duncan wrote:
>> > I feel that Perl 5.36.0 SHOULD fix its own broken behavior
>> regarding stack
>> > traces.
>> >
>> > The 5.36.0 release is a major release and is far enough away that
>> DBIx::Class
>> > should be able to get fixed to not rely on the broken behavior in
>> time for its
>> > release.
>> >
>> > The fix shouldn't be put off another year. If anyone depends on
>> something that
>> > depends on the broken behavior, they should not upgrade past 5.34.x
>> until their
>> > other dependencies are updated for compatibility.
>> >
>> > I can live with that.
>> >
>> > I am not really convinced the patch really does break anything however,
>> we have
>> > been running with the fix backported for years and we never noticed any
>> issues
>> > with our DBIx class code. *shrug*
>>
>> That's all the more reason to re-apply the bug fix to blead right away.
>>
>> Your experience would imply that any reliance in DBIx::Class on the
>> broken
>> behavior isn't in its core functionality and more in some non-core
>> functionality
>> or alternately just a test is broken and main code isn't.
>>
>
> That would be my intuition, but I don't know that we use enough DBIx to be
> certain. But it's extremely hard for me to understand how such broken data
> could ever be truly useful.
>
> Yves
>
>>
>>

On Fri, Jan 14, 2022 at 1:15 PM demerphq <demerphq@gmail.com> wrote:

> On Fri, 14 Jan 2022, 20:00 Darren Duncan, <darren@darrenduncan.net> wrote:
>
>> On 2022-01-14 3:40 a.m., demerphq wrote:
>> > On Fri, 14 Jan 2022 at 11:57, Darren Duncan wrote:
>> > I feel that Perl 5.36.0 SHOULD fix its own broken behavior
>> regarding stack
>> > traces.
>> >
>> > The 5.36.0 release is a major release and is far enough away that
>> DBIx::Class
>> > should be able to get fixed to not rely on the broken behavior in
>> time for its
>> > release.
>> >
>> > The fix shouldn't be put off another year. If anyone depends on
>> something that
>> > depends on the broken behavior, they should not upgrade past 5.34.x
>> until their
>> > other dependencies are updated for compatibility.
>> >
>> > I can live with that.
>> >
>> > I am not really convinced the patch really does break anything however,
>> we have
>> > been running with the fix backported for years and we never noticed any
>> issues
>> > with our DBIx class code. *shrug*
>>
>> That's all the more reason to re-apply the bug fix to blead right away.
>>
>> Your experience would imply that any reliance in DBIx::Class on the
>> broken
>> behavior isn't in its core functionality and more in some non-core
>> functionality
>> or alternately just a test is broken and main code isn't.
>>
>
> That would be my intuition, but I don't know that we use enough DBIx to be
> certain. But it's extremely hard for me to understand how such broken data
> could ever be truly useful.
>

What breaks is a test that checks what modules are loaded by DBIx::Class.
The change in behavior causes it to make different conclusions as to who is
loading what. I suspect this is representative for the sort of code that
would be affected by this change.

Leon

On Fri, 14 Jan 2022, 21:27 Leon Timmermans, <fawaka@gmail.com> wrote:

> On Fri, Jan 14, 2022 at 1:15 PM demerphq <demerphq@gmail.com> wrote:
>
>> On Fri, 14 Jan 2022, 20:00 Darren Duncan, <darren@darrenduncan.net>
>> wrote:
>>
>>> On 2022-01-14 3:40 a.m., demerphq wrote:
>>> > On Fri, 14 Jan 2022 at 11:57, Darren Duncan wrote:
>>> > I feel that Perl 5.36.0 SHOULD fix its own broken behavior
>>> regarding stack
>>> > traces.
>>> >
>>> > The 5.36.0 release is a major release and is far enough away that
>>> DBIx::Class
>>> > should be able to get fixed to not rely on the broken behavior in
>>> time for its
>>> > release.
>>> >
>>> > The fix shouldn't be put off another year. If anyone depends on
>>> something that
>>> > depends on the broken behavior, they should not upgrade past
>>> 5.34.x until their
>>> > other dependencies are updated for compatibility.
>>> >
>>> > I can live with that.
>>> >
>>> > I am not really convinced the patch really does break anything
>>> however, we have
>>> > been running with the fix backported for years and we never noticed
>>> any issues
>>> > with our DBIx class code. *shrug*
>>>
>>> That's all the more reason to re-apply the bug fix to blead right away.
>>>
>>> Your experience would imply that any reliance in DBIx::Class on the
>>> broken
>>> behavior isn't in its core functionality and more in some non-core
>>> functionality
>>> or alternately just a test is broken and main code isn't.
>>>
>>
>> That would be my intuition, but I don't know that we use enough DBIx to
>> be certain. But it's extremely hard for me to understand how such broken
>> data could ever be truly useful.
>>
>
> What breaks is a test that checks what modules are loaded by DBIx::Class.
> The change in behavior causes it to make different conclusions as to who is
> loading what. I suspect this is representative for the sort of code that
> would be affected by this change.
>


But those assumptions are basically broken, so I'm not sure your point.
Classic case of GIGO.

Yves

>

On Fri, Jan 14, 2022 at 3:33 PM demerphq <demerphq@gmail.com> wrote:

> What breaks is a test that checks what modules are loaded by DBIx::Class.
>> The change in behavior causes it to make different conclusions as to who is
>> loading what. I suspect this is representative for the sort of code that
>> would be affected by this change.
>>
>
> But those assumptions are basically broken, so I'm not sure your point.
> Classic case of GIGO.
>

It's not broken if it gets the job done, is it?

I agree with you that the new behavior is far more usable than the older
behavior, but that doesn't mean people haven't put the old behavior to use.

Leon

On Friday, 14 January 2022, 15:54:27 CET, Leon Timmermans <fawaka@gmail.com> wrote:

> > But those assumptions are basically broken, so I'm not sure your point. Classic case of GIGO.
>
> It's not broken if it gets the job done, is it?

> I agree with you that the new behavior is far more usable than the older behavior, but that doesn't
> mean people haven't put the old behavior to use.

I think the old behavior kind of worked because generally, stack traces work. It appears to be the use/require case which is problematic.

However, in the use/require case, if the apparently pseudo-random ordering is deterministic, on my machine the first stack frame grabbed happens to be correct. If that holds (pretty sure it must), then for the 'import' case where you're trying to figure out where you're exporting to, you're going to export those functions to the right spot. It's when you hit stack traces that it becomes an unusable mess.

As it stands:

* The stack trace is clearly incorrect
* That makes it useless for debugging
* When you're working on a *huge* system and get those traces, it's miserable

Given that, I suspect the most useful discussions are about *how* to fix it and/or *when* to fix it..

Best,
Ovid
-- 
IT consulting, training, specializing in Perl, databases, and agile development
http://www.allaroundtheworld.fr/. 


Buy my book! - http://bit.ly/beginning_perl

On Fri, Jan 14, 2022 at 4:59 PM Ovid <curtis_ovid_poe@yahoo.com> wrote:

> On Friday, 14 January 2022, 15:54:27 CET, Leon Timmermans <
> fawaka@gmail.com> wrote:
>
> > > But those assumptions are basically broken, so I'm not sure your
> point. Classic case of GIGO.
> >
> > It's not broken if it gets the job done, is it?
>
> > I agree with you that the new behavior is far more usable than the older
> behavior, but that doesn't
> > mean people haven't put the old behavior to use.
>
> I think the old behavior kind of worked because generally, stack traces
> work. It appears to be the use/require case which is problematic.
>
> However, in the use/require case, if the apparently pseudo-random ordering
> is deterministic, on my machine the first stack frame grabbed happens to be
> correct. If that holds (pretty sure it must), then for the 'import' case
> where you're trying to figure out where you're exporting to, you're going
> to export those functions to the right spot. It's when you hit stack traces
> that it becomes an unusable mess.
>
> As it stands:
>
> * The stack trace is clearly incorrect
> * That makes it useless for debugging
> * When you're working on a *huge* system and get those traces, it's
> miserable
>
> Given that, I suspect the most useful discussions are about *how* to fix
> it and/or *when* to fix it.
>

"How to fix it" probably includes people stepping up to deal with the
fallout. IME the most useful thing proponents of this change could do is
actually write patches for the 3 known affected modules. I don't really
understand how that hasn't happened yet in the past two years.

Leon

On Sat, 15 Jan 2022, 00:45 Leon Timmermans, <fawaka@gmail.com> wrote:

> On Fri, Jan 14, 2022 at 4:59 PM Ovid <curtis_ovid_poe@yahoo.com> wrote:
>
>> On Friday, 14 January 2022, 15:54:27 CET, Leon Timmermans <
>> fawaka@gmail.com> wrote:
>>
>> > > But those assumptions are basically broken, so I'm not sure your
>> point. Classic case of GIGO.
>> >
>> > It's not broken if it gets the job done, is it?
>>
>> > I agree with you that the new behavior is far more usable than the
>> older behavior, but that doesn't
>> > mean people haven't put the old behavior to use.
>>
>> I think the old behavior kind of worked because generally, stack traces
>> work. It appears to be the use/require case which is problematic.
>>
>> However, in the use/require case, if the apparently pseudo-random
>> ordering is deterministic, on my machine the first stack frame grabbed
>> happens to be correct. If that holds (pretty sure it must), then for the
>> 'import' case where you're trying to figure out where you're exporting to,
>> you're going to export those functions to the right spot. It's when you hit
>> stack traces that it becomes an unusable mess.
>>
>> As it stands:
>>
>> * The stack trace is clearly incorrect
>> * That makes it useless for debugging
>> * When you're working on a *huge* system and get those traces, it's
>> miserable
>>
>> Given that, I suspect the most useful discussions are about *how* to fix
>> it and/or *when* to fix it.
>>
>
> "How to fix it" probably includes people stepping up to deal with the
> fallout. IME the most useful thing proponents of this change could do is
> actually write patches for the 3 known affected modules. I don't really
> understand how that hasn't happened yet in the past two years.
>

I think there is a feeling that this is just like people baking a
dependency on hash order into their tests. All I saw on this subject was
people claiming the obviously broken behavior was "correct" and they
wouldn't accept that the change had a reasonable foundation. I disengaged
after people started arguing that the stack trace claiming things were used
on lines where they clearly weren't was correct and reasonable. There is to
much to do in life than to argue with people who vehemently insist that up
is down. Furthermore I didn't see anybody demonstrate a logical issue that
was clearly a problem, just broken and badly designed tests.

Anyway I'll find some time to dig into the DBIx part, but I don't know what
the other two are. If you do it would save me some time digging through the
email backlogs.

Yves

>

On Fri, Jan 14, 2022 at 5:44 PM Leon Timmermans <fawaka@gmail.com> wrote:

> On Fri, Jan 14, 2022 at 4:59 PM Ovid <curtis_ovid_poe@yahoo.com> wrote:
>
>> On Friday, 14 January 2022, 15:54:27 CET, Leon Timmermans <
>> fawaka@gmail.com> wrote:
>>
>> > > But those assumptions are basically broken, so I'm not sure your
>> point. Classic case of GIGO.
>> >
>> > It's not broken if it gets the job done, is it?
>>
>> > I agree with you that the new behavior is far more usable than the
>> older behavior, but that doesn't
>> > mean people haven't put the old behavior to use.
>>
>> I think the old behavior kind of worked because generally, stack traces
>> work. It appears to be the use/require case which is problematic.
>>
>> However, in the use/require case, if the apparently pseudo-random
>> ordering is deterministic, on my machine the first stack frame grabbed
>> happens to be correct. If that holds (pretty sure it must), then for the
>> 'import' case where you're trying to figure out where you're exporting to,
>> you're going to export those functions to the right spot. It's when you hit
>> stack traces that it becomes an unusable mess.
>>
>> As it stands:
>>
>> * The stack trace is clearly incorrect
>> * That makes it useless for debugging
>> * When you're working on a *huge* system and get those traces, it's
>> miserable
>>
>> Given that, I suspect the most useful discussions are about *how* to fix
>> it and/or *when* to fix it.
>>
>
> "How to fix it" probably includes people stepping up to deal with the
> fallout. IME the most useful thing proponents of this change could do is
> actually write patches for the 3 known affected modules. I don't really
> understand how that hasn't happened yet in the past two years.
>

I think I just did that for DBIx::Class.

Leon

On Sat, 15 Jan 2022 at 05:18, Leon Timmermans <fawaka@gmail.com> wrote:

> On Fri, Jan 14, 2022 at 5:44 PM Leon Timmermans <fawaka@gmail.com> wrote:
>
>> On Fri, Jan 14, 2022 at 4:59 PM Ovid <curtis_ovid_poe@yahoo.com> wrote:
>>
>>> On Friday, 14 January 2022, 15:54:27 CET, Leon Timmermans <
>>> fawaka@gmail.com> wrote:
>>>
>>> > > But those assumptions are basically broken, so I'm not sure your
>>> point. Classic case of GIGO.
>>> >
>>> > It's not broken if it gets the job done, is it?
>>>
>>> > I agree with you that the new behavior is far more usable than the
>>> older behavior, but that doesn't
>>> > mean people haven't put the old behavior to use.
>>>
>>> I think the old behavior kind of worked because generally, stack traces
>>> work. It appears to be the use/require case which is problematic.
>>>
>>> However, in the use/require case, if the apparently pseudo-random
>>> ordering is deterministic, on my machine the first stack frame grabbed
>>> happens to be correct. If that holds (pretty sure it must), then for the
>>> 'import' case where you're trying to figure out where you're exporting to,
>>> you're going to export those functions to the right spot. It's when you hit
>>> stack traces that it becomes an unusable mess.
>>>
>>> As it stands:
>>>
>>> * The stack trace is clearly incorrect
>>> * That makes it useless for debugging
>>> * When you're working on a *huge* system and get those traces, it's
>>> miserable
>>>
>>> Given that, I suspect the most useful discussions are about *how* to fix
>>> it and/or *when* to fix it.
>>>
>>
>> "How to fix it" probably includes people stepping up to deal with the
>> fallout. IME the most useful thing proponents of this change could do is
>> actually write patches for the 3 known affected modules. I don't really
>> understand how that hasn't happened yet in the past two years.
>>
>
> I think I just did that for DBIx::Class.
>
>
Thanks, although I feel it is worth noting that this was not a fix to
DBIx::Class and the patch never broke DBIx::Class, it was the tests for
DBIx::Class that were broken only.

cheers,
Yves

--
perl -Mre=debug -e "/just|another|perl|hacker/"

On Sat, 15 Jan 2022 at 07:06, demerphq <demerphq@gmail.com> wrote:

> On Sat, 15 Jan 2022 at 05:18, Leon Timmermans <fawaka@gmail.com> wrote:
>
>> On Fri, Jan 14, 2022 at 5:44 PM Leon Timmermans <fawaka@gmail.com> wrote:
>>
>>> On Fri, Jan 14, 2022 at 4:59 PM Ovid <curtis_ovid_poe@yahoo.com> wrote:
>>>
>>>> On Friday, 14 January 2022, 15:54:27 CET, Leon Timmermans <
>>>> fawaka@gmail.com> wrote:
>>>>
>>>> > > But those assumptions are basically broken, so I'm not sure your
>>>> point. Classic case of GIGO.
>>>> >
>>>> > It's not broken if it gets the job done, is it?
>>>>
>>>> > I agree with you that the new behavior is far more usable than the
>>>> older behavior, but that doesn't
>>>> > mean people haven't put the old behavior to use.
>>>>
>>>> I think the old behavior kind of worked because generally, stack traces
>>>> work. It appears to be the use/require case which is problematic.
>>>>
>>>> However, in the use/require case, if the apparently pseudo-random
>>>> ordering is deterministic, on my machine the first stack frame grabbed
>>>> happens to be correct. If that holds (pretty sure it must), then for the
>>>> 'import' case where you're trying to figure out where you're exporting to,
>>>> you're going to export those functions to the right spot. It's when you hit
>>>> stack traces that it becomes an unusable mess.
>>>>
>>>> As it stands:
>>>>
>>>> * The stack trace is clearly incorrect
>>>> * That makes it useless for debugging
>>>> * When you're working on a *huge* system and get those traces, it's
>>>> miserable
>>>>
>>>> Given that, I suspect the most useful discussions are about *how* to
>>>> fix it and/or *when* to fix it.
>>>>
>>>
>>> "How to fix it" probably includes people stepping up to deal with the
>>> fallout. IME the most useful thing proponents of this change could do is
>>> actually write patches for the 3 known affected modules. I don't really
>>> understand how that hasn't happened yet in the past two years.
>>>
>>
>> I think I just did that for DBIx::Class.
>>
>>
> Thanks, although I feel it is worth noting that this was not a fix to
> DBIx::Class and the patch never broke DBIx::Class, it was the tests for
> DBIx::Class that were broken only.
>
> I have looked at the other two cases and as far as I can tell Dave M did
actually dig into them and fix them. They are in the thread here:

https://github.com/Perl/perl5/issues/17663

BTW, in that thread you said: "This ticket raises the question «what is
correctness». The current behavior is truthful, but not intuitive. This
discussion really seems to be about which of those two values are more
relevant for correctness."

Consider the following:

~/tlib$ cat {A..D}.pm
package A;
BEGIN { Carp::cluck "\n--\nIn ", __PACKAGE__; }
use B;
1;
__END__


package B;
BEGIN { Carp::cluck "\n--\nIn ", __PACKAGE__; }
use C;
1;
__END__


package C;
BEGIN { Carp::cluck "\n--\nIn ", __PACKAGE__; }
use D;
1;
__END__


package D;
BEGIN { Carp::cluck "\n--\nIn ", __PACKAGE__; }
1;
__END__


~/tlib$ perl -I. -MCarp -e'use A;'

--
In A at A.pm line 2.
A::BEGIN() called at A.pm line 2
eval {...} called at A.pm line 2
require A.pm called at -e line 1
main::BEGIN() called at A.pm line 2
eval {...} called at A.pm line 2

--
In B at B.pm line 2.
B::BEGIN() called at B.pm line 2
eval {...} called at B.pm line 2
require B.pm called at A.pm line 3
A::BEGIN() called at B.pm line 2
eval {...} called at B.pm line 2
require A.pm called at -e line 1
main::BEGIN() called at B.pm line 2
eval {...} called at B.pm line 2

--
In C at C.pm line 2.
C::BEGIN() called at C.pm line 2
eval {...} called at C.pm line 2
require C.pm called at B.pm line 3
B::BEGIN() called at C.pm line 2
eval {...} called at C.pm line 2
require B.pm called at A.pm line 3
A::BEGIN() called at C.pm line 2
eval {...} called at C.pm line 2
require A.pm called at -e line 1
main::BEGIN() called at C.pm line 2
eval {...} called at C.pm line 2

--
In D at D.pm line 2.
D::BEGIN() called at D.pm line 2
eval {...} called at D.pm line 2
require D.pm called at C.pm line 3
C::BEGIN() called at D.pm line 2
eval {...} called at D.pm line 2
require C.pm called at B.pm line 3
B::BEGIN() called at D.pm line 2
eval {...} called at D.pm line 2
require B.pm called at A.pm line 3
A::BEGIN() called at D.pm line 2
eval {...} called at D.pm line 2
require A.pm called at -e line 1
main::BEGIN() called at D.pm line 2
eval {...} called at D.pm line 2

So respectfully I would like to know how you can consider such
contradictory results (eg the same stack frame being attributed to
different lines of code depending on when caller was called) can be
truthful, especially given some of the stack frames are BEGIN's which fire
once and are never fired again, it seems to me that we should be able to
agree that such a construct should be shown to come from exactly one place,
and one place alone.

Cheers,
Yves

On Sat, Jan 15, 2022 at 8:22 AM demerphq <demerphq@gmail.com> wrote:

> On Sat, 15 Jan 2022 at 07:06, demerphq <demerphq@gmail.com> wrote:
>
>> On Sat, 15 Jan 2022 at 05:18, Leon Timmermans <fawaka@gmail.com> wrote:
>>
>>> On Fri, Jan 14, 2022 at 5:44 PM Leon Timmermans <fawaka@gmail.com>
>>> wrote:
>>>
>>>> On Fri, Jan 14, 2022 at 4:59 PM Ovid <curtis_ovid_poe@yahoo.com> wrote:
>>>>
>>>>> On Friday, 14 January 2022, 15:54:27 CET, Leon Timmermans <
>>>>> fawaka@gmail.com> wrote:
>>>>>
>>>>> > > But those assumptions are basically broken, so I'm not sure your
>>>>> point. Classic case of GIGO.
>>>>> >
>>>>> > It's not broken if it gets the job done, is it?
>>>>>
>>>>> > I agree with you that the new behavior is far more usable than the
>>>>> older behavior, but that doesn't
>>>>> > mean people haven't put the old behavior to use.
>>>>>
>>>>> I think the old behavior kind of worked because generally, stack
>>>>> traces work. It appears to be the use/require case which is problematic.
>>>>>
>>>>> However, in the use/require case, if the apparently pseudo-random
>>>>> ordering is deterministic, on my machine the first stack frame grabbed
>>>>> happens to be correct. If that holds (pretty sure it must), then for the
>>>>> 'import' case where you're trying to figure out where you're exporting to,
>>>>> you're going to export those functions to the right spot. It's when you hit
>>>>> stack traces that it becomes an unusable mess.
>>>>>
>>>>> As it stands:
>>>>>
>>>>> * The stack trace is clearly incorrect
>>>>> * That makes it useless for debugging
>>>>> * When you're working on a *huge* system and get those traces, it's
>>>>> miserable
>>>>>
>>>>> Given that, I suspect the most useful discussions are about *how* to
>>>>> fix it and/or *when* to fix it.
>>>>>
>>>>
>>>> "How to fix it" probably includes people stepping up to deal with the
>>>> fallout. IME the most useful thing proponents of this change could do is
>>>> actually write patches for the 3 known affected modules. I don't really
>>>> understand how that hasn't happened yet in the past two years.
>>>>
>>>
>>> I think I just did that for DBIx::Class.
>>>
>>>
>> Thanks, although I feel it is worth noting that this was not a fix to
>> DBIx::Class and the patch never broke DBIx::Class, it was the tests for
>> DBIx::Class that were broken only.
>>
>> I have looked at the other two cases and as far as I can tell Dave M did
> actually dig into them and fix them. They are in the thread here:
>
> https://github.com/Perl/perl5/issues/17663
>
> BTW, in that thread you said: "This ticket raises the question «what is
> correctness». The current behavior is truthful, but not intuitive. This
> discussion really seems to be about which of those two values are more
> relevant for correctness."
>
> Consider the following:
>
> ...
>
> So respectfully I would like to know how you can consider such
> contradictory results (eg the same stack frame being attributed to
> different lines of code depending on when caller was called) can be
> truthful, especially given some of the stack frames are BEGIN's which fire
> once and are never fired again, it seems to me that we should be able to
> agree that such a construct should be shown to come from exactly one place,
> and one place alone.
>

As far as I can tell, it's an accurate representation of how the sausage is
made. BEGIN blocks aren't implemented the way one would expect them to, and
these stacktraces represent that. They're not false, just very unhelpful.

Leon

On Fri, Jan 14, 2022 at 10:06 PM demerphq <demerphq@gmail.com> wrote:

> Thanks, although I feel it is worth noting that this was not a fix to
> DBIx::Class and the patch never broke DBIx::Class, it was the tests for
> DBIx::Class that were broken only.
>

I don't think that's very fair to say. The tests are verifying behaviour
expected by the runtime code, and there is a reason why they are written
the way they are. Otherwise, any broken test could be "fixed" by simply
removing the test altogether. We've all worked with junior programmers who
want to do that, and we've all rightfully scolded them for it.

Part of fixing the test is understanding why it was written the way it was,
and what runtime behaviour is associated with that test. Changing a test's
expected result means that runtime behaviour is also changing, and other
things might need to be addressed there as a result.

On the other hand, it could be that this test isn't testing anything useful
at all, but I don't think that's been established here.

On 1/15/22 19:01, Karen Etheridge wrote:

>
>
> On Fri, Jan 14, 2022 at 10:06 PM demerphq <demerphq@gmail.com> wrote:
>
> Thanks, although I feel it is worth noting that this was not a fix
> to DBIx::Class and the patch never broke DBIx::Class, it was the
> tests for DBIx::Class that were broken only.
>
> I don't think that's very fair to say. The tests are verifying
> behaviour expected by the runtime code, and there is a reason why they
> are written the way they are. Otherwise, any broken test could be
> "fixed" by simply removing the test altogether. We've all worked with
> junior programmers who want to do that, and we've all rightfully
> scolded them for it.
>
> Part of fixing the test is understanding why it was written the way it
> was, and what runtime behaviour is associated with that test. Changing
> a test's expected result means that runtime behaviour is also
> changing, and other things might need to be addressed there as a result.
>
> On the other hand, it could be that this test isn't testing anything
> useful at all, but I don't think that's been established here.
>

Or the test tests something useful, works around bugs to do so, and when
those bugs get fixed, the test starts failing. It looks like that is the
case here.


HTH,

M4

On Sat, 15 Jan 2022 at 14:27, Leon Timmermans <fawaka@gmail.com> wrote:

> On Sat, Jan 15, 2022 at 8:22 AM demerphq <demerphq@gmail.com> wrote:
>
>>
>> As far as I can tell, it's an accurate representation of how the sausage
> is made. BEGIN blocks aren't implemented the way one would expect them to,
> and these stacktraces represent that. They're not false, just very
> unhelpful.
>

I really don't understand your position on this. We see output that says 4
contradictory things, none of which are actually accurate. So which one of
the four is true and which ones are false? I dont think the following
statements can all be true simultaneously.

main::BEGIN() called at A.pm line 2
main::BEGIN() called at B.pm line 2
main::BEGIN() called at C.pm line 2
main::BEGIN() called at D.pm line 2

But main::BEGIN is actually called at -e line 1. I mean, if the BEGIN
statement were created in a module it would would A::BEGIN or B::BEGIN. For
example:

$ perl -MCarp=cluck -le'package Foo; BEGIN { Carp::cluck("in begin") }'
in begin at -e line 1.
Foo::BEGIN() called at -e line 1
eval {...} called at -e line 1

I really think this is an important point that keeps getting lost. Sure you
can say "this is how the sausage is made", as this is the result of using a
single global to track the origin of ALL synthesized BEGIN blocks which are
part of a use statement. But that is circular logic. "it is correct and
true because that is how it works" could be used to justify leaving unfixed
ANY bug we encounter.

If the output did not say "called at A.pm line 2" but said "fake-frame
injected by compiler" or something like that, I could buy the "this is how
the sausage is made" argument. But it doesn't. It says something different
each time. None of which seem to have any relationship to reality.

It seems to me we should be able to agree on whether the output is true,
and whether it is correct. I am really struggling to understand why this is
controversial. It seems to me we should be able to agree that main::BEGIN
*cannot* be called from anywhere but main, and this means that the output
claiming that the block was called from a module file cannot be correct.

Cheers,
Yves

--
perl -Mre=debug -e "/just|another|perl|hacker/"

On Sat, 15 Jan 2022 at 19:01, Karen Etheridge <perl@froods.org> wrote:

>
>
> On Fri, Jan 14, 2022 at 10:06 PM demerphq <demerphq@gmail.com> wrote:
>
>> Thanks, although I feel it is worth noting that this was not a fix to
>> DBIx::Class and the patch never broke DBIx::Class, it was the tests for
>> DBIx::Class that were broken only.
>>
>
> I don't think that's very fair to say.
>

Fair is a question of subjective opinion. Correctness should be of
universal agreement. If something isnt correct it is broken.

Would you take this position if someone built a dependency on the exact
hash order returned by an old Perl?

Yves

On Sat, 15 Jan 2022 at 20:20, Martijn Lievaart <m@rtij.nl> wrote:

> On 1/15/22 19:01, Karen Etheridge wrote:
>
>
>
> On Fri, Jan 14, 2022 at 10:06 PM demerphq <demerphq@gmail.com> wrote:
>
>> Thanks, although I feel it is worth noting that this was not a fix to
>> DBIx::Class and the patch never broke DBIx::Class, it was the tests for
>> DBIx::Class that were broken only.
>>
>
> I don't think that's very fair to say. The tests are verifying behaviour
> expected by the runtime code, and there is a reason why they are written
> the way they are. Otherwise, any broken test could be "fixed" by simply
> removing the test altogether. We've all worked with junior programmers who
> want to do that, and we've all rightfully scolded them for it.
>
> Part of fixing the test is understanding why it was written the way it
> was, and what runtime behaviour is associated with that test. Changing a
> test's expected result means that runtime behaviour is also changing, and
> other things might need to be addressed there as a result.
>
> On the other hand, it could be that this test isn't testing anything
> useful at all, but I don't think that's been established here.
>
>
> Or the test tests something useful, works around bugs to do so, and when
> those bugs get fixed, the test starts failing. It looks like that is the
> case here.
>
That doesn't change the fact that the *test* was broken by this change, NOT
the code it was testing. Fixing the test to pass under the caller() fix did
not require modifying anything but the test code, and as such I feel
entirely entirely to say that the *test* was broken, not the code.

cheers,
Yves

--
perl -Mre=debug -e "/just|another|perl|hacker/"

On Sun, Jan 16, 2022 at 1:54 AM demerphq <demerphq@gmail.com> wrote:

> On Sat, 15 Jan 2022 at 14:27, Leon Timmermans <fawaka@gmail.com> wrote:
>
>> On Sat, Jan 15, 2022 at 8:22 AM demerphq <demerphq@gmail.com> wrote:
>>
>>>
>>> As far as I can tell, it's an accurate representation of how the sausage
>> is made. BEGIN blocks aren't implemented the way one would expect them to,
>> and these stacktraces represent that. They're not false, just very
>> unhelpful.
>>
>
> I really don't understand your position on this. We see output that says 4
> contradictory things, none of which are actually accurate. So which one of
> the four is true and which ones are false? I dont think the following
> statements can all be true simultaneously.
>
> main::BEGIN() called at A.pm line 2
> main::BEGIN() called at B.pm line 2
> main::BEGIN() called at C.pm line 2
> main::BEGIN() called at D.pm line 2
>
> But main::BEGIN is actually called at -e line 1. I mean, if the BEGIN
> statement were created in a module it would would A::BEGIN or B::BEGIN. For
> example:
>
> $ perl -MCarp=cluck -le'package Foo; BEGIN { Carp::cluck("in begin") }'
> in begin at -e line 1.
> Foo::BEGIN() called at -e line 1
> eval {...} called at -e line 1
>
> I really think this is an important point that keeps getting lost. Sure
> you can say "this is how the sausage is made", as this is the result of
> using a single global to track the origin of ALL synthesized BEGIN blocks
> which are part of a use statement. But that is circular logic. "it is
> correct and true because that is how it works" could be used to justify
> leaving unfixed ANY bug we encounter.
>

No, it is specific to this case because we are talking about a feature that
itself describes how Perl works. This logic would not be relevant for most
features.


> If the output did not say "called at A.pm line 2" but said "fake-frame
> injected by compiler" or something like that, I could buy the "this is how
> the sausage is made" argument. But it doesn't. It says something different
> each time. None of which seem to have any relationship to reality.
>
> It seems to me we should be able to agree on whether the output is true,
> and whether it is correct. I am really struggling to understand why this is
> controversial. It seems to me we should be able to agree that main::BEGIN
> *cannot* be called from anywhere but main, and this means that the output
> claiming that the block was called from a module file cannot be correct.
>

No one is claiming it is the most useful thing to display to the user. Just
that it represents what the Perl interpreter sees, at least to some extent.
The only thing controversial is various declarations of what is correct and
what must be done, where a reasoned discussion and assumption of good faith
would be much more effective at motivating resolution of your issue.

-Dan

On 2022-01-15 11:10 p.m., Dan Book wrote:
> On Sun, Jan 16, 2022 at 1:54 AM demerphq wrote:
> It seems to me we should be able to agree on whether the output is true, and
> whether it is correct. I am really struggling to understand why this is
> controversial. It seems to me we should be able to agree that main::BEGIN
> *cannot* be called from anywhere but main, and this means that the output
> claiming that the block was called from a module file cannot be correct.
>
> No one is claiming it is the most useful thing to display to the user. Just that
> it represents what the Perl interpreter sees, at least to some extent. The only
> thing controversial is various declarations of what is correct and what must be
> done, where a reasoned discussion and assumption of good faith would be much
> more effective at motivating resolution of your issue.

Perhaps the best solution here is reporting the most comprehensive result that
gives all of the information that either of the behaviors under discussion
gives, but does so in a deterministic manner.

So include the information about BEGIN blocks but have the result structured in
a way that makes it easy to filter that out if we don't want it. So those who
desire either the older or newer version of the output have all the info they
need to derive it.

Alternately, while it may be considered a bit kludgy, maybe leave caller alone
with the behavior it has had for decades and add a NEW complementary built-in
that provides the other behavior, and then those that want the alternate "more
correct" behavior can call the new one instead.

-- Darren Duncan

On Sun, 16 Jan 2022, 15:10 Dan Book, <grinnz@gmail.com> wrote:

> On Sun, Jan 16, 2022 at 1:54 AM demerphq <demerphq@gmail.com> wrote:
>
>> On Sat, 15 Jan 2022 at 14:27, Leon Timmermans <fawaka@gmail.com> wrote:
>>
>>> On Sat, Jan 15, 2022 at 8:22 AM demerphq <demerphq@gmail.com> wrote:
>>>
>>>>
>>>> As far as I can tell, it's an accurate representation of how the
>>> sausage is made. BEGIN blocks aren't implemented the way one would expect
>>> them to, and these stacktraces represent that. They're not false, just very
>>> unhelpful.
>>>
>>
>> I really don't understand your position on this. We see output that says
>> 4 contradictory things, none of which are actually accurate. So which one
>> of the four is true and which ones are false? I dont think the following
>> statements can all be true simultaneously.
>>
>> main::BEGIN() called at A.pm line 2
>> main::BEGIN() called at B.pm line 2
>> main::BEGIN() called at C.pm line 2
>> main::BEGIN() called at D.pm line 2
>>
>> But main::BEGIN is actually called at -e line 1. I mean, if the BEGIN
>> statement were created in a module it would would A::BEGIN or B::BEGIN. For
>> example:
>>
>> $ perl -MCarp=cluck -le'package Foo; BEGIN { Carp::cluck("in begin") }'
>> in begin at -e line 1.
>> Foo::BEGIN() called at -e line 1
>> eval {...} called at -e line 1
>>
>> I really think this is an important point that keeps getting lost. Sure
>> you can say "this is how the sausage is made", as this is the result of
>> using a single global to track the origin of ALL synthesized BEGIN blocks
>> which are part of a use statement. But that is circular logic. "it is
>> correct and true because that is how it works" could be used to justify
>> leaving unfixed ANY bug we encounter.
>>
>
> No, it is specific to this case because we are talking about a feature
> that itself describes how Perl works. This logic would not be relevant for
> most features.
>

I don't understand this. See below.


>
>> If the output did not say "called at A.pm line 2" but said "fake-frame
>> injected by compiler" or something like that, I could buy the "this is how
>> the sausage is made" argument. But it doesn't. It says something different
>> each time. None of which seem to have any relationship to reality.
>>
>> It seems to me we should be able to agree on whether the output is true,
>> and whether it is correct. I am really struggling to understand why this is
>> controversial. It seems to me we should be able to agree that main::BEGIN
>> *cannot* be called from anywhere but main, and this means that the output
>> claiming that the block was called from a module file cannot be correct.
>>
>
> No one is claiming it is the most useful
>

Useful is a matter of opinion I would like to stay away from. The question
is correctness.

I don't see why just because this is about how the internals work that we
can't talk about correctness and must restrict ourselves to usefulness,
especially as I am arguing the internals aren't working properly given
expected results.

There are many places within my domain of expertise in the internals where
perl does something that I agree to be incorrect even though it's a "won't
fix" because fixing it is intractable given how the internals work. We/I
don't pretend the behaviour is correct because that is how things work, we
accept they are bugs which we can't or won't fix. Why is this case
different? Admittedly there are cases where the answer is unclear, but I
dont feel that the class of bugs we see from caller() is one of them.

thing to display to the user. Just that it represents what the Perl
> interpreter sees, at least to some extent. The only thing controversial is
> various declarations of what is correct and what must be done, where a
> reasoned discussion and assumption of good faith would be much more
> effective at motivating resolution of your issue.
>

I really don't appreciate being accused of acting with bad faith. Nothing I
have said was intended to be disrespectful nor as far as I can tell was
disrespectful. If anyone perceives it that way I do apologize in advance
and would be open to discuss it in more detail offline and learn how i
could have expressed my position without being accused of bad faith.

It seems to me that achieving consensus that this behaviour is buggy or not
is required to move forward here. I cannot think of another case where if a
perl function returned different results for the same question (rand aside)
depending on where and when it was called would NOT be a bug.

If it is not a bug we shouldn't change it. If it is a bug we should.
Obviously I feel it is a bug and I have stated two reasons. Those claiming
it's not seem don't seem to address those points at all. I don't consider
"because that is how perl works" a reasonable answer for the simple fact
that that argument can be applied to any bug in perl.

To recap, my arguments this is a bug are as follows (assuming the code
posted below):

1.
The output shows that:
A::BEGIN() called at D.pm line 2
A::BEGIN() called at C.pm line 2
A::BEGIN() called at B.pm line 2
A::BEGIN() called at A.pm line 2

How can this be true at the same time given the code posted?

2.
Line 2 of A.pm, B.pm, C.pm D.pm are all identical and do not include a use
statement:
BEGIN { Carp::cluck "\n--\nIn ", __PACKAGE__; }

Even if we assume there is an off-by-one error in the line number and say
it is referring to line 3 of the respective modules we see that line 3 of
each is as follows:

A.pm line 3: use B;
B.pm line 3: use C;
C.pm line 3: use D;
D.pm line 3: 1;

So how can caller be returning a true and correct result when it says that
says A::BEGIN was called from D.pm line 3?

3.
Which part of that documentation for caller() explains the current
behaviour? The current behaviour seems to be in direct contravention of the
first three sentences of the documentation of the function:

Returns the context of the current pure perl subroutine call. In
scalar context, returns the caller's package name if there is a
caller (that is, if we're in a subroutine or "eval" or
"require")
and the undefined value otherwise. caller never returns XS subs
and they are skipped. The next pure perl sub will appear instead
of the XS sub in caller's return values. In list context, caller
returns

# 0 1 2
my ($package, $filename, $line) = caller;

So IMO we have to conclude there is a bug here, either in the docs, or in
the implementation. Occams razor would suggest it is the implementation not
the docs.

Cheers,
Yves
~/tlib$ cat {A..D}.pm
package A;
BEGIN { Carp::cluck "\n--\nIn ", __PACKAGE__; }
use B;
1;
__END__


package B;
BEGIN { Carp::cluck "\n--\nIn ", __PACKAGE__; }
use C;
1;
__END__


package C;
BEGIN { Carp::cluck "\n--\nIn ", __PACKAGE__; }
use D;
1;
__END__


package D;
BEGIN { Carp::cluck "\n--\nIn ", __PACKAGE__; }
1;
__END__


~/tlib$ perl -I. -MCarp -e'use A;'

--
In A at A.pm line 2.
A::BEGIN() called at A.pm line 2
eval {...} called at A.pm line 2
require A.pm called at -e line 1
main::BEGIN() called at A.pm line 2
eval {...} called at A.pm line 2

--
In B at B.pm line 2.
B::BEGIN() called at B.pm line 2
eval {...} called at B.pm line 2
require B.pm called at A.pm line 3
A::BEGIN() called at B.pm line 2
eval {...} called at B.pm line 2
require A.pm called at -e line 1
main::BEGIN() called at B.pm line 2
eval {...} called at B.pm line 2

--
In C at C.pm line 2.
C::BEGIN() called at C.pm line 2
eval {...} called at C.pm line 2
require C.pm called at B.pm line 3
B::BEGIN() called at C.pm line 2
eval {...} called at C.pm line 2
require B.pm called at A.pm line 3
A::BEGIN() called at C.pm line 2
eval {...} called at C.pm line 2
require A.pm called at -e line 1
main::BEGIN() called at C.pm line 2
eval {...} called at C.pm line 2

--
In D at D.pm line 2.
D::BEGIN() called at D.pm line 2
eval {...} called at D.pm line 2
require D.pm called at C.pm line 3
C::BEGIN() called at D.pm line 2
eval {...} called at D.pm line 2
require C.pm called at B.pm line 3
B::BEGIN() called at D.pm line 2
eval {...} called at D.pm line 2
require B.pm called at A.pm line 3
A::BEGIN() called at D.pm line 2
eval {...} called at D.pm line 2
require A.pm called at -e line 1
main::BEGIN() called at D.pm line 2
eval {...} called at D.pm line 2

>

On Sun, Jan 16, 2022 at 3:32 AM demerphq <demerphq@gmail.com> wrote:

>
>
> On Sun, 16 Jan 2022, 15:10 Dan Book, <grinnz@gmail.com> wrote:
>
>> On Sun, Jan 16, 2022 at 1:54 AM demerphq <demerphq@gmail.com> wrote:
>>
>>> On Sat, 15 Jan 2022 at 14:27, Leon Timmermans <fawaka@gmail.com> wrote:
>>>
>>>> On Sat, Jan 15, 2022 at 8:22 AM demerphq <demerphq@gmail.com> wrote:
>>>>
>>>>>
>>>>> As far as I can tell, it's an accurate representation of how the
>>>> sausage is made. BEGIN blocks aren't implemented the way one would expect
>>>> them to, and these stacktraces represent that. They're not false, just very
>>>> unhelpful.
>>>>
>>>
>>> I really don't understand your position on this. We see output that says
>>> 4 contradictory things, none of which are actually accurate. So which one
>>> of the four is true and which ones are false? I dont think the following
>>> statements can all be true simultaneously.
>>>
>>> main::BEGIN() called at A.pm line 2
>>> main::BEGIN() called at B.pm line 2
>>> main::BEGIN() called at C.pm line 2
>>> main::BEGIN() called at D.pm line 2
>>>
>>> But main::BEGIN is actually called at -e line 1. I mean, if the BEGIN
>>> statement were created in a module it would would A::BEGIN or B::BEGIN. For
>>> example:
>>>
>>> $ perl -MCarp=cluck -le'package Foo; BEGIN { Carp::cluck("in begin") }'
>>> in begin at -e line 1.
>>> Foo::BEGIN() called at -e line 1
>>> eval {...} called at -e line 1
>>>
>>> I really think this is an important point that keeps getting lost. Sure
>>> you can say "this is how the sausage is made", as this is the result of
>>> using a single global to track the origin of ALL synthesized BEGIN blocks
>>> which are part of a use statement. But that is circular logic. "it is
>>> correct and true because that is how it works" could be used to justify
>>> leaving unfixed ANY bug we encounter.
>>>
>>
>> No, it is specific to this case because we are talking about a feature
>> that itself describes how Perl works. This logic would not be relevant for
>> most features.
>>
>
> I don't understand this. See below.
>
>
>>
>>> If the output did not say "called at A.pm line 2" but said "fake-frame
>>> injected by compiler" or something like that, I could buy the "this is how
>>> the sausage is made" argument. But it doesn't. It says something different
>>> each time. None of which seem to have any relationship to reality.
>>>
>>> It seems to me we should be able to agree on whether the output is true,
>>> and whether it is correct. I am really struggling to understand why this is
>>> controversial. It seems to me we should be able to agree that main::BEGIN
>>> *cannot* be called from anywhere but main, and this means that the output
>>> claiming that the block was called from a module file cannot be correct.
>>>
>>
>> No one is claiming it is the most useful
>>
>
> Useful is a matter of opinion I would like to stay away from. The question
> is correctness.
>
> I don't see why just because this is about how the internals work that we
> can't talk about correctness and must restrict ourselves to usefulness,
> especially as I am arguing the internals aren't working properly given
> expected results.
>
> There are many places within my domain of expertise in the internals where
> perl does something that I agree to be incorrect even though it's a "won't
> fix" because fixing it is intractable given how the internals work. We/I
> don't pretend the behaviour is correct because that is how things work, we
> accept they are bugs which we can't or won't fix. Why is this case
> different? Admittedly there are cases where the answer is unclear, but I
> dont feel that the class of bugs we see from caller() is one of them.
>
> thing to display to the user. Just that it represents what the Perl
>> interpreter sees, at least to some extent. The only thing controversial is
>> various declarations of what is correct and what must be done, where a
>> reasoned discussion and assumption of good faith would be much more
>> effective at motivating resolution of your issue.
>>
>
> I really don't appreciate being accused of acting with bad faith. Nothing
> I have said was intended to be disrespectful nor as far as I can tell was
> disrespectful. If anyone perceives it that way I do apologize in advance
> and would be open to discuss it in more detail offline and learn how i
> could have expressed my position without being accused of bad faith.
>

To be clear: I did not accuse you of this. An assumption of good faith
should be made of the participants in the discussion, and I appreciate that
you are now asking for clarification.

-Dan

On Thu, Jan 13, 2022, at 12:04 AM, demerphq wrote:
> This subject has come up multiple times and is a source of deep frustration for me, and deep confusion for the many non-perl developers who work on perl code at Booking.com.
>
> This was fixed by Dave Mitchell in f2f32cd638746f538da6db804dab6dd54e654f30
> https://github.com/Perl/perl5/commit/f2f32cd638746f538da6db804dab6dd54e654f30
>
> This patch was then reverted in merge commit c0d05305c156c83e4f9f3a207451b3175fbb7f24.

Hello, I am joining this thread way up at the top, because the bottom looks like it's gotten pretty sticky.

There has been a lot of debate over something that I think, here, is inconsequential: whether it is accurate to call the current output "correct." I don't think we are going to get to the universal agreement on that without a lot more arguing, and* I don't think it buys us anything*.

On the other hand, I think everybody basically agrees that the "fixed" behavior is preferable, modulo some downstream inconvenience. Some test suites will need to be fixed, etc. In fact, the idea seemed to be that "somebody" would look into this come 5.33.0, but it didn't happen, because sometimes we all get busy.

So, should we have caller output that accurately reflects the relationship of the caller to the source document? *Yes.*

I have re-opened #15109 <https://github.com/Perl/perl5/issues/15109>.

Should we apply that fix immediately? *I don't know.*

What other CPAN code is affected by this change? "Somebody" can run a CPAN smoke, but not me, any time soon. To get there, we'll want to revert the revert, which at present apply cleanly. If we smoke CPAN, see that a handful of things are fixed, and can provide fixes or at least point at how they'd be done, I feel okay about trying to ship this change this year.

If it's twenty high-value modules and exceedingly complex to fix, I think we should get it in a branch and flag it to be pushed on throughout 2022 so we can ship the fix in 5.38.0.

Would anybody like to take responsibility for getting a branch and CPAN smoke going?

--
rjbs

On Sun, Jan 16, 2022 at 02:40:09PM -0500, Ricardo Signes wrote:
> Hello, I am joining this thread way up at the top, because the bottom
> looks like it's gotten pretty sticky.

Hello, thought I'd join you up here where the air is clear and the views
spectacular!

> On the other hand, I think everybody basically agrees that the "fixed"
> behavior is preferable

ribasushi, shadowcat-mst and Vincent Pit in the ticket for 'BBC breaks
DBIx:Class' (#17663), all seemed to think my patch was in some way
philosophically wrong, but to varying degrees agreed to work around it if
we insisted on (eventually) un-reverting it.

So I think we could do with a bit more discussion, which I will seed
below. But my opinion so far is that my patch is correct and should be
applied, but directly after 5.36.0 is released, to give distro owners max
time to fix things up.

First off, much of the discussion in that ticket is very confused/ing, but
various issues are being conflated.

First there is a very specific bug, which makes stack traces into garbage,
whereby if there are nested BEGIN/requires (a.k.a use's) then if the code
croaks within the innermost nested file, the stack backtrace shows *all*
the nested requires etc as having been done in the *lower-most* Foo.pm file,
destroying most of the information that should be contained in the
backtrace, and making it very confusing even for experienced coders.

i.e. the stacktrack looks (in part) like

require called at C.pm line 7
require called at C.pm line 7
require called at C.pm line 7

when you'd expect it to look (in part) like

require called at A.pm line 245
require called at B.pm line 35
require called at C.pm line 7

Technically speaking, this is due to same COP being used for all requires,
so when the current COP gets its file/line number set, all the things up
the stack see the same change. I don't think (at least I hope) that
anyone disagrees that this is a bug.

My fix for this also changed the general behaviour of how BEGIN reports
its caller - this is the bit which upset various people and broke various
distributions. This is the second issue. I happen to think that my fix was
correct here too, and indeed allows us to close ticket .

To an extent this is a philosophical issue: caller(0) identifies the call
site of who who called us; i.e. in

sub foo { @c = caller(0); print "@c" }
...
foo(); # call site

we rightly expect caller(0) to identify the file and line containing the
'foo()' call, and identify what package name was in force when that line
was compiled (and also what hints, warnings, hints hash etc were active at
the same time).

The trouble comes from when the sub is a BEGIN - who is the caller of the
sub? What file and line number? What package? What hints and warnings were
in scope?

This can be demonstrated with some simple code, first with the output from
normal perl, and second with perl including my patch:

p:
#!/usr/bin/perl
package X;
use Foo; # equivalent to BEGIN { require Foo.pm }

Foo.pm
package Foo;
use Carp;
confess("in Foo");


$ perl5340 ~/tmp/p
in Foo at /tmp/lib/Foo.pm line 3.
require Foo.pm called at /home/davem/tmp/p line 3
X::BEGIN() called at /tmp/lib/Foo.pm line 0
eval {...} called at /tmp/lib/Foo.pm line 0

$ patched-perl ~/tmp/p
in Foo at /tmp/lib/Foo.pm line 3.
require Foo.pm called at /home/davem/tmp/p line 3
X::BEGIN() called at /home/davem/tmp/p line 3
eval {...} called at /home/davem/tmp/p line 3


To me, it seems trivially clear that current perl is wrong and my patch
fixes it. But the argument that mst and others made (which I didn't
entirely follow) seems to be that, whereas I see the call site for the
BEGIN to reside in the source file at the line at the end of BEGIN, they
see it as being wherever perl was last executing (I think?), which they
seemed to think was the require/eval. But that confused me, because BEGIN
isn't always associated with a require - so what do we do then? Or do they
think that in that case, the 'line 0' stuff is in fact correct?

While most of the modules listed as broken were described in the #17663
BBC ticket, there was another BBC report attached to #15109, for
Devel::CompileLevel, whose breakage was more confusing - the module was
modifying ${^WARNING_BITS} at compile time, then failing to find those
modification where it expected on the stack. I haven't looked into that
again yet (if ever) because it's taken me long enough today just to
remind myself of what the main issues were in ticket #17663. But I kind
of trust my former self who said it was a bug in the module.


--
1 - number of times I have needed to use the cap lock key
10000 - number of times I have CCIDENTLY HIT THE CAPS KEY WHEN TYPING 'A'

On Fri, 21 Jan 2022 at 17:19, Dave Mitchell <davem@iabyn.com> wrote:

> On Sun, Jan 16, 2022 at 02:40:09PM -0500, Ricardo Signes wrote:
> > Hello, I am joining this thread way up at the top, because the bottom
> > looks like it's gotten pretty sticky.
>
> Hello, thought I'd join you up here where the air is clear and the views
> spectacular!
>
> > On the other hand, I think everybody basically agrees that the "fixed"
> > behavior is preferable
>
> ribasushi, shadowcat-mst and Vincent Pit in the ticket for 'BBC breaks
> DBIx:Class' (#17663), all seemed to think my patch was in some way
> philosophically wrong, but to varying degrees agreed to work around it if
> we insisted on (eventually) un-reverting it.
>
> So I think we could do with a bit more discussion, which I will seed
> below. But my opinion so far is that my patch is correct and should be
> applied, but directly after 5.36.0 is released, to give distro owners max
> time to fix things up.
>

I would like to get it released in 5.36.0 as a build option. I know people
who would use that build option and are backporting your original patch and
if there are conflicts I would like to get it resolved. More specifically
Booking has been backporting your patch ever since you wrote it, to perls
from before when you wrote it. Thus a long time, (with no issue mind you).
Even though I don't work at Booking any more I would like to get this
sorted out so my friends and colleagues there do not have to deal with it.

I would be happy to write the required code.

cheers,
Yves

On Mon, 14 Feb 2022 at 04:26, demerphq <demerphq@gmail.com> wrote:

> On Fri, 21 Jan 2022 at 17:19, Dave Mitchell <davem@iabyn.com> wrote:
>
>> On Sun, Jan 16, 2022 at 02:40:09PM -0500, Ricardo Signes wrote:
>> > Hello, I am joining this thread way up at the top, because the bottom
>> > looks like it's gotten pretty sticky.
>>
>> Hello, thought I'd join you up here where the air is clear and the views
>> spectacular!
>>
>> > On the other hand, I think everybody basically agrees that the "fixed"
>> > behavior is preferable
>>
>> ribasushi, shadowcat-mst and Vincent Pit in the ticket for 'BBC breaks
>> DBIx:Class' (#17663), all seemed to think my patch was in some way
>> philosophically wrong, but to varying degrees agreed to work around it if
>> we insisted on (eventually) un-reverting it.
>>
>> So I think we could do with a bit more discussion, which I will seed
>> below. But my opinion so far is that my patch is correct and should be
>> applied, but directly after 5.36.0 is released, to give distro owners max
>> time to fix things up.
>>
>
> I would like to get it released in 5.36.0 as a build option. I know people
> who would use that build option and are backporting your original patch and
> if there are conflicts I would like to get it resolved. More specifically
> Booking has been backporting your patch ever since you wrote it, to perls
> from before when you wrote it. Thus a long time, (with no issue mind you).
> Even though I don't work at Booking any more I would like to get this
> sorted out so my friends and colleagues there do not have to deal with it.
>
> I would be happy to write the required code.
>

For now I have created

https://github.com/Perl/perl5/pull/19415

which reverts the revert, with conflicts resolved. I kinda wonder if we
should reduce it to two patches however, one was a fix for the other, if we
reapply we might as well squash them down.

I have not implemented a define flag to enable the corrected behaviour yet.

Yves

--
perl -Mre=debug -e "/just|another|perl|hacker/"