builtin::reftype() should, like ref(), always return a string

[PhilipGuenther]

Module: builtin

Description
One of the great things about ref() is that it always returns a
string, meaning that it's easy to use---even under 'warnings'---without
having to perform other tests on the argument or result first. Even
ref(undef) returns a defined value.

Contrast builtin::reftype(), which returns undef for non-references.
This makes it harder to use in code that may operate on mixed scalars
and references (tree walks, etc).

This also renders the advice in the UNIVERSAL(3) pod incorrect,
where it says:

 Previous versions of this documentation suggested using "isa"
 as a function to determine the type of a reference:

   $yes = UNIVERSAL::isa($h, "HASH");
   $yes = UNIVERSAL::isa("Foo", "Bar");

 The problem is that this code would never call an overridden
 "isa" method in any class.  Instead, use "reftype" from
 Scalar::Util for the first case:

   use Scalar::Util 'reftype';

   $yes = reftype( $h ) eq "HASH";

This works without warnings when $h isn't a reference:
$yes = UNIVERSAL::isa($h, "HASH");

This doesn't:
$yes = reftype( $h ) eq "HASH";

You have to use one of these instead:
$yes = ref($h) ne '' && reftype( $h ) eq "HASH";
$yes = defined reftype( $h ) && reftype( $h ) eq "HASH";
$yes = (reftype( $h ) // '') eq "HASH";
$yes = do { my $t = reftype( $h ); defined $t && $t eq "HASH" };

Having ref() return '' for non-references may be unexpected, but
it has great UX. reftype() should follow that.

Steps to Reproduce

---

use v5.40;

my($scalar, $undef) = ( 'foo' );

these are good

if (ref($scalar) eq 'HASH') { say "process hash"; }
if (ref($undef) eq 'ARRAY') { say "process array"; }

these warn

if (reftype($scalar) eq 'HASH') { say "process hash"; }
if (reftype($undef) eq 'ARRAY') { say "process array"; }

say "scalar = $scalar";

Expected behavior
Script should output "scalar = foo", but instead emits warnings to get this:

Use of uninitialized value $scalar in string eq at - line 10.
Use of uninitialized value $undef in string eq at - line 11.
scalar = foo

(side note: that first error is misleading/confusing to claim that
$scalar is uninitialized when it clearly contains 'foo')

Perl configuration

Site configuration information for perl 5.40.1:

Configured by root at Thu Jan  1  0:00:00 UTC 1970.

Summary of my perl5 (revision 5 version 40 subversion 1) configuration:
   
  Platform:
    osname=openbsd
    osvers=7.6
    archname=amd64-openbsd
    uname='openbsd'
    config_args='-dse -Dopenbsd_distribution=defined -Dmksymlinks'
    hint=recommended
    useposix=true
    d_sigaction=define
    useithreads=undef
    usemultiplicity=undef
    use64bitint=define
    use64bitall=define
    uselongdouble=undef
    usemymalloc=n
    default_inc_excludes_dot=define
  Compiler:
    cc='cc'
    ccflags ='-DNO_LOCALE_NUMERIC -DNO_LOCALE_COLLATE -DNO_LOCALE_NUMERIC -DNO_LOCALE_COLLATE -DNO_LOCALE_MONETARY -DNO_
LOCALE_TIME -DNO_LOCALE_MESSAGES -DLIBC_HANDLES_MISMATCHED_CTYPE -fno-strict-aliasing -fno-delete-null-pointer-checks -p
ipe -fstack-protector-strong -I/usr/local/include'
    optimize='-O2'
    cppflags='-DBIG_TIME -DNO_LOCALE_NUMERIC -DNO_LOCALE_COLLATE -DNO_LOCALE_NUMERIC -DNO_LOCALE_COLLATE -DNO_LOCALE_MON
ETARY -DNO_LOCALE_TIME -DNO_LOCALE_MESSAGES -DLIBC_HANDLES_MISMATCHED_CTYPE -fno-strict-aliasing -fno-delete-null-pointe
r-checks -pipe -fstack-protector-strong -I/usr/local/include'
    ccversion=''
    gccversion='OpenBSD Clang 16.0.6'
    gccosandvers=''
    intsize=4
    longsize=8
    ptrsize=8
    doublesize=8
    byteorder=12345678
    doublekind=3
    d_longlong=define
    longlongsize=8
    d_longdbl=define
    longdblsize=16
    longdblkind=3
    ivtype='long'
    ivsize=8
    nvtype='double'
    nvsize=8
    Off_t='off_t'
    lseeksize=8
    alignbytes=8
    prototype=define
  Linker and Libraries:
    ld='cc'
    ldflags ='-Wl,-E  -fstack-protector-strong -L/usr/local/lib'
    libpth=/usr/lib /usr/lib/clang/16/lib
    libs=-lm -lc
    perllibs=-lm -lc
    libc=/usr/lib/libc.so.100.3
    so=so
    useshrplib=true
    libperl=libperl.so.25.0
    gnulibc_version=''
  Dynamic Linking:
    dlsrc=dl_dlopen.xs
    dlext=so
    d_dlsymun=undef
    ccdlflags='-Wl,-R/usr/libdata/perl5/amd64-openbsd/CORE'
    cccdlflags='-DPIC -fpic '
    lddlflags='-shared -fpic  -fstack-protector-strong -L/usr/local/lib'


---
@INC for perl 5.40.1:
    /usr/local/libdata/perl5/site_perl/amd64-openbsd
    /usr/local/libdata/perl5/site_perl
    /usr/libdata/perl5/amd64-openbsd
    /usr/libdata/perl5

---
Environment for perl 5.40.1:
    HOME=/home/users/guenther
    LANG (unset)
    LANGUAGE (unset)
    LC_CTYPE=C.UTF-8
    LD_LIBRARY_PATH (unset)
    LOGDIR (unset)
    PATH=/home/users/guenther/bin:/bin:/usr/bin:/usr/local/bin:/usr/local/sbin:/sbin:/usr/sbin:/usr/X11R6/bin:/usr/games
    PERL_BADLANG (unset)
    SHELL=/bin/ksh

[Grinnz]

I agree that it would be better if it returned an empty string instead of undef for non-references. Unfortunately I don't think that would be reasonable to change without a feature flag or new function name, as defined reftype $scalar would have been a reasonable check to do with the current functionality. We also should keep Scalar::Util::reftype's behavior the same as builtin::reftype to avoid confusion.

[PhilipGuenther]

I can understand closing this as Won't Fix. I get that once it left experimental, this was really unlikely to change.

Two requests:

• at least fix the EXPORTS section of the UNIVERSAL(3) pod to give a correct replacement for UNIVERSAL::isa($h, "HASH")
• count this as a vote for builtin::reftype string comparison provides incorrect warning #19273 to fix reftype's argument incorrectly appearing in "uninitialized value" warning

I've added "don't assume returning undef from non-boolean scalar functions is a good default" to my own code review/style list; not sure if there would have enough consensus to share more broadly or if that's, well, just style. :)

[bulk88]

This ticket sounds related to #23395 and
#23391 . defined() && has real-time overhead to silence a stupid use warnings or use strict console warning.

no warnings; and a new {} block is an even worse solution because the warnings check code/macros/bitfields/COP/RCPV/refcounted he code is so deoptimized for no-warning runloop execution. To make a long story short, you've already created the printf string for the console warning before the perl engine realizes to abort sending that buffer to STDERR. No warnings is actually slower than the defined test and it's 2-4 pp funcs from the last time I benchmarked it.

A ton of optimization opportunities exist here.

[leonerd]

On the surface of it, yes it might makes sense to have these operations return empty strings rather than undef. But there's a deeper issue.

A longer-term thought about ref and friends is that it shouldn't just return a string, but rather a full "type object" that is itself a data structure that can be accessed, etc.. An empty string wouldn't make sense here, whereas undef would.

The problem is therefore much more an issue with the use of eq that complains about undef. This isn't just a problem with ref/reftype, but occurs in all sorts of other situations too. A better way to solve this is to have a string equality test operator that doesn't complain about undef, and doesn't believe that undef is equal to the empty string plus that warning.

This is the motivation behind PPC0030. Regretably I have not yet had time to implement this in time for Perl 5.42, but I hope to get around to it in the round leading up to (5.)44.