perl5 icon indicating copy to clipboard operation
perl5 copied to clipboard

Imported lexicals don't warn of redefinition

Open leonerd opened this issue 2 years ago • 6 comments

Consider the redefinition warning in:

$ bleadperl -Mwarnings -E 'use builtin "true"; sub true { "surprise" }  say true()'
Prototype mismatch: sub true () vs none at -e line 1.
Subroutine true redefined at -e line 1.
surprise

This is what we want. But done in the other order, no redefinition warning happens (besides the experimental one that #21330 will soon remove)

$ bleadperl -Mwarnings -E 'sub true { "surprise" }  use builtin "true"; say true()'
Built-in function 'builtin::true' is experimental at -e line 1.
1

I would like to have seen a warning here; either the same "redefined" one or something specifically worded about lexical imports; something such as:

Imported subroutine "true" overrides previously defined subroutine at -e line 1.

leonerd avatar Aug 16 '23 15:08 leonerd

The warning should probably match the existing warning for lexicals:

"state" subroutine &true masks earlier declaration in same scope

And it should only apply when masking lexicals, not package subs.

haarg avatar Aug 16 '23 16:08 haarg

Mmm possibly; though that raises the issue that it claims to be a "state" subroutine in the first place. It ought not be really - I just found I had to turn on the STATE flag otherwise something crashed somewhere. I never really dug into why at the time. Perhaps we should fix that too while we're here?

leonerd avatar Aug 16 '23 16:08 leonerd

The warning should probably match the existing warning for lexicals:

"state" subroutine &true masks earlier declaration in same scope

It already does:

$ perl -wE 'state sub true {} use builtin "true"'
"state" subroutine &true masks earlier declaration in same scope at -e line 1.

ilmari avatar Aug 16 '23 16:08 ilmari

It already does:

Oh oops. Mistake when testing.

I don't think there's anything to change here then, unless the general handling of lexical subs changes.

haarg avatar Aug 16 '23 16:08 haarg

You get the same warning if the original sub is just lexical too, I would find it confusing as an end user that this mentions state at all. If we manage to turn off the state flag then I think all would be fine in terms of expected warning.

$ perl -wE 'my sub true {} use builtin "true"'
"state" subroutine &true masks earlier declaration in same scope at -e line 1.

JRaspass avatar Aug 16 '23 19:08 JRaspass

Further observations: Perl does not warn when a lexical subroutine overshadows a package one. For example:

$ ./perl -wce 'sub true() {}  my sub true() {}'
-e syntax OK

Therefore it would seem a little odd if use builtin's lexical imports would warn here, because those lexical imports are just doing the moral equivalent. It's all a bit of a mess, in summary.

leonerd avatar Jan 17 '24 18:01 leonerd