Thank you once again for your bug report, investigating it has been
quite an adventure.
The quick answer to your bug report is that Aspect is actually behaving
correctly, but that I have done a terrible job of explaining why in the
documentation.
To understand the long answer, firstly please note that the rewrite of
Aspect from the original codebase I took over still has the
Aspect::Point tree of modules to go, so there's still a few
implementation nigglies in this area.
I'll address your problem in two parts.
Firstly, the after_returning case.
One of the main responsibilities of Aspect is to ensure that regardless
of what is done in the advice code, the resulting call to the original
function matches as closely as possible the original code.
This extends to the list context of the original call.
In your bug report, you suggest the following use case.
query_person('007');
sub query_person {
my ($id) = @_;
my $person = 'James Bond';
return $person;
}
However, this is NOT code that you would ever actually use, as you are
calling the function in void context.
In practice, you would actually call the following.
my $person = query_person('007');
When Aspect calls the original function, it calls it in the same
context. So if your call is in scalar context, it ensures that the call
to the original function is also in scalar context, and captures the
return value.
Because you called query_person() in void context, Aspect also called
the original function in void context, resulting in no return value, and
thus the undef you observed.
This is still somewhat wrong behaviour. Ideally it should throw an
exception along the lines of "No return_value in void context". But this
would mean every single user had to check ->wantarray first before
risking a call to ->return_value because you don't control the context
you are called in. As a result, I had previously judged it preferable to
just have return_value return undef.
I may readdress this assumption when I complete the rewrite of the
Aspect::Point tree.
To summarise, while your example did NOT work, the following WILL work
as you expect.
use Aspect;
use Data::Dumper;
after_returning {
printf "tracing: %s(%s)\n", $_->sub_name,
join ', ', @{$_->{'params'}};
## $_->proceed;
printf "\treturn %s\n", $_->return_value;
print Dumper($_);
} call qr/query_person/;
my $person = query_person('007');
sub query_person {
my ($id) = @_;
my $person = 'James Bond';
return $person;
}
Secondly, let me address the around() case.
In the original version of Aspect that I took over, the use of proceed
was different to the usage of it in the AspectJ toolkit (which I have
been trying to broadly mimic where possible).
While the AspectJ "proceed" triggers the actual execution of the
original code, in Aspect.pm it is used in before() advice as a boolean
to indicate "should I proceed after this advice".
This is NOT a meaning that makes sense for around() advice, where the
triggering of the original function must be immediate.
To compensate, the original author created a ->run_original method which
achieves the effect you want, plus a ->original access if you
absolutely, positively need to call the original code directly without
context, param, exception or return_value safety.
It was my mistake to show "proceed" in the documentation, as it is my
intention to make proceed do what you expect after the Aspect::Point
rewrite and I got ahead of myself when writing the documentation.
I have corrected the documentation to say "run_original".
With regards to your original test case, the following is the final
corrected functionality you are plooking for.
use Aspect;
use Data::Dumper;
around {
printf "tracing: %s(%s)\n", $_->sub_name,
join ', ', @{$_->{'params'}};
$_->run_original;
printf "\treturn %s\n", $_->return_value;
print Dumper($_);
} call qr/query_person/;
my $person = query_person('007');
sub query_person {
my ($id) = @_;
my $person = 'James Bond';
return $person;
}
To resolve this problem, I have corrected the documentation and in the
rewrite (which I hope to achieve over Christmas) I shall change the
behaviour of proceed in around() advice to do what you mean, with
run_original eventually being deprecated.
Best Regards
Adam Kennedy
