From 44f09b433ec852b4d7744b782fe56d7fd6dcf55a Mon Sep 17 00:00:00 2001
From: "Paul \"LeoNerd\" Evans" <leonerd@leonerd.org.uk>
Date: Thu, 25 Jan 2024 20:25:55 +0000
Subject: [PATCH] Make builtin import idempotent thus avoiding some questions
 about multiple symbols

---
 builtin.c              | 70 +++++++++++++++++++++++-------------------
 lib/builtin-unimport.t | 11 +++++++
 2 files changed, 50 insertions(+), 31 deletions(-)

diff --git a/builtin.c b/builtin.c
index 415c84d00d27c..9ee83e85ad412 100644
--- a/builtin.c
+++ b/builtin.c
@@ -584,19 +584,6 @@ static bool S_parse_version(const char *vstr, const char *vend, UV *vmajor, UV *
     return TRUE;
 }
 
-#define import_sym(sym)  S_import_sym(aTHX_ sym)
-static void S_import_sym(pTHX_ SV *sym)
-{
-    SV *ampname = sv_2mortal(Perl_newSVpvf(aTHX_ "&%" SVf, SVfARG(sym)));
-    SV *fqname = sv_2mortal(Perl_newSVpvf(aTHX_ "builtin::%" SVf, SVfARG(sym)));
-
-    CV *cv = get_cv(SvPV_nolen(fqname), SvUTF8(fqname) ? SVf_UTF8 : 0);
-    if(!cv)
-        Perl_croak(aTHX_ builtin_not_recognised, sym);
-
-    export_lexical(ampname, (SV *)cv);
-}
-
 #define cv_is_builtin(cv)  S_cv_is_builtin(aTHX_ cv)
 static bool S_cv_is_builtin(pTHX_ CV *cv)
 {
@@ -604,30 +591,51 @@ static bool S_cv_is_builtin(pTHX_ CV *cv)
     return file && strEQ(file, __FILE__);
 }
 
+enum {
+    BEHAVIOUR_IMPORT,
+    BEHAVIOUR_TOMBSTONE,
+};
+
+#define import_or_tombstone_sym(sym, behaviour)  S_import_or_tombstone_sym(aTHX_ sym, behaviour)
+static void S_import_or_tombstone_sym(pTHX_ SV *sym, int behaviour)
+{
+    SV *ampname = sv_2mortal(Perl_newSVpvf(aTHX_ "&%" SVf, SVfARG(sym)));
+
+    bool got = false;
+    PADOFFSET off = pad_findmy_sv(ampname, 0);
+    CV *cv;
+    if(off != NOT_IN_PAD &&
+            SvTYPE((cv = (CV *)PL_curpad[off])) == SVt_PVCV &&
+            cv_is_builtin(cv))
+        got = true;
+
+    if(!got && behaviour == BEHAVIOUR_IMPORT) {
+        SV *fqname = sv_2mortal(Perl_newSVpvf(aTHX_ "builtin::%" SVf, SVfARG(sym)));
+
+        CV *cv = get_cv(SvPV_nolen(fqname), SvUTF8(fqname) ? SVf_UTF8 : 0);
+        if(!cv)
+            Perl_croak(aTHX_ builtin_not_recognised, sym);
+
+        export_lexical(ampname, (SV *)cv);
+    }
+    else if(got && behaviour == BEHAVIOUR_TOMBSTONE) {
+        pad_add_name_sv(ampname, padadd_STATE|padadd_TOMBSTONE, 0, 0);
+    }
+}
+
 void
 Perl_import_builtin_bundle(pTHX_ U16 ver, bool do_unimport)
 {
-    SV *ampname = sv_newmortal();
+    SV *symname = sv_newmortal();
 
     for(int i = 0; builtins[i].name; i++) {
-        sv_setpvf(ampname, "&%s", builtins[i].name);
+        sv_setpvf(symname, "%s", builtins[i].name);
 
         bool want = (builtins[i].since_ver <= ver);
-
-        bool got = false;
-        PADOFFSET off = pad_findmy_sv(ampname, 0);
-        CV *cv;
-        if(off != NOT_IN_PAD &&
-                SvTYPE((cv = (CV *)PL_curpad[off])) == SVt_PVCV &&
-                cv_is_builtin(cv))
-            got = true;
-
-        if(!got && want) {
-            import_sym(newSVpvn_flags(builtins[i].name, strlen(builtins[i].name), SVs_TEMP));
-        }
-        else if(do_unimport && got && !want) {
-            pad_add_name_sv(ampname, padadd_STATE|padadd_TOMBSTONE, 0, 0);
-        }
+        if(want)
+            import_or_tombstone_sym(symname, BEHAVIOUR_IMPORT);
+        else if(do_unimport)
+            import_or_tombstone_sym(symname, BEHAVIOUR_TOMBSTONE);
     }
 }
 
@@ -667,7 +675,7 @@ XS(XS_builtin_import)
             continue;
         }
 
-        import_sym(sym);
+        import_or_tombstone_sym(sym, BEHAVIOUR_IMPORT);
     }
 
     finish_export_lexical();
diff --git a/lib/builtin-unimport.t b/lib/builtin-unimport.t
index a7921d2fc021b..4ed7daccaeb59 100644
--- a/lib/builtin-unimport.t
+++ b/lib/builtin-unimport.t
@@ -34,6 +34,17 @@ no warnings 'experimental::builtin';
     }
 }
 
+# multiple imports are idempotent
+{
+    use builtin 'true';
+    use builtin 'true';
+    is(true(), "1", 'imported true() from two use lines');
+
+    no builtin 'true';
+    ok(!defined eval "true(); 1", 'true() is no longer visible after a single no');
+    like($@, qr/^Undefined subroutine &main::true called at /, 'Failure from missing function');
+}
+
 # vim: tabstop=4 shiftwidth=4 expandtab autoindent softtabstop=4
 
 done_testing();