Decide on how to use (or not to use) Perl function prototypes or signatures

[stephanegigandet]

Is your feature request related to a problem? Please describe.

In most of the Product Opener code, we use Perl prototypes to specify how many arguments functions have.

e.g.

sub product_url($) {

	my $code_or_ref = shift;

This is something I believe is useful, because if the function is called with a different number of arguments, it will fail at compile time.

e.g. if I add this function in Display.pm

sub print_my_product_url() {

	print(product_url(1234, 12));
}

Then when I do "make restart", the container stops and I have this error in the logs:

[Fri Jun 24 09:48:12.380196 2022] [perl:error] [pid 16] Too many arguments for ProductOpener::Products::product_url at /opt/product-opener/lib/ProductOpener/Products.pm line 2436, near "12)"\nCompilation failed in require at /opt/product-opener/lib/ProductOpener/Users.pm line 91.\nBEGIN failed--compilation aborted at /opt/product-opener/lib/ProductOpener/Users.pm line 91.\nCompilation failed in require at /opt/product-opener/lib/ProductOpener/Display.pm line 147.\nBEGIN failed--compilation aborted at /opt/product-opener/lib/ProductOpener/Display.pm line 147.\nCompilation failed in require at /opt/product-opener/lib/startup_apache2.pl line 66.\nBEGIN failed--compilation aborted at /opt/product-opener/lib/startup_apache2.pl line 66.\nCompilation failed in require at (eval 2) line 1.\n
[Fri Jun 24 09:48:12.380239 2022] [perl:error] [pid 16] Can't load Perl file: /opt/product-opener/lib/startup_apache2.pl for server openfoodfacts.localhost:0, exiting...

But there are lots of threads on the Web that explain that Perl's prototypes are harmful / should not be used. e.g. https://stackoverflow.com/questions/297034/why-are-perl-5s-function-prototypes-bad

Describe the solution you'd like

I think we could just use the "experimental" signatures of Perl (that are in fact not experimental anymore in the just released Perl 5.36):

https://perldoc.pl/perlsub#Signatures

Any thoughts?

cc @hangy @alexgarel @zigouras

Describe alternatives you've considered

No response

Additional context

No response

Number of products impacted

No response

Time per product

No response

[smonff]

Hi Stéphane,

Answering here after reading the message on FR Perl Mongers mailing list.

use feature qw(signatures);
no warnings qw(experimental::signatures);

The boilerplate pain can be reduced easily by using the trick described in Ovid's Enforcing Simple Standards with One Module blog post. This blog also gives interesting information about companies making the subroutines feature switch.

One more reason to adopt: signatures went out of the experimental stage with perl 5.36 and declared a stable feature. Personally, I use them since they appeared (with 5.20), I don't have any complaints to express.

[hangy]

One more reason to adopt: signatures went out of the experimental stage with perl 5.36 and declared a stable feature.

That's great news IMHO. Using experimental features in a larger codebase can be bad, but using signatures would make the code more pragmatic.

[stephanegigandet]

Hi @smonff , thanks a lot for your reply! The Veure::Module is interesting, maybe we could just use our own version of it (I couldn't find the Veure one on cpan so I assume it's internal).

[hangy]

The Veure::Module is interesting, maybe we could just use our own version of it (I couldn't find the Veure one on cpan so I assume it's internal).

The source code looks to be in the blog post. I think we have something (partly) similar with the use of Modern::Perl, but we can definitely also add this like a ProductOpener::PerlStandards module.

[Ban3]

I think we have something (partly) similar with the use of Modern::Perl

Ovid does mention this in the post:

Modern::Perl is a nice middle ground for avoiding this, but it may not be the features you want.

Looks like Modern::Perl handles signature importing if we bump the feature to 5.34

We could indeed go for our own module if we have something to add to Modern::Perl, like the utf8 boilerplate.

[stephanegigandet]

Looks like Modern::Perl handles signature importing if we bump the feature to 5.34

I never noticed how small the code for Modern::Perl is. :)

I'd be in favour of just switching to:

use Modern::Perl '2022';

So that new programmers don't have to look at what we put in our own module (which would just be "use utf8;" I guess).

[smonff]

The most important aspect here is Import::Into.

If you create your own ProductOpener::Module, on the model of
Veure::Module (assuming this is reusable code, this one being only an example, not a distributed CPAN module indeed), you could save the pain of adding the signatures boilerplate all over the place.

[stephanegigandet]

Good news: the Perl signatures in their current form were introduced in Perl 5.24, which is the old Perl we have in production today. :)

[stephanegigandet]

But "use Modern::Perl '2022'" does not work if Perl is not 5.34.

Feature bundle "5.34" is not supported by Perl 5.28.1 at /opt/perl/local/lib/perl5/Modern/Perl.pm line 37.

So I guess we just create our own Modern::Perl / Veure::Module then.

[stephanegigandet]

I'll try it and make a PR for it.

[Ban3]

I never noticed how small the code for Modern::Perl is. :)

Yeah, it's basically just

warnings->import;
strict->import;
feature->import( $feature_tag )

Just add signatures to that, and we have our own module.

[stephanegigandet]

How does this look?
#7009

[alexgarel]

We can close this, most code is migrated.