Make builtin import idempotent thus avoiding some questions about mul…

…tiple symbols
leonerd · Jan 25, 2024 · 44f09b4 · 44f09b4
1 parent e365299
commit 44f09b4
Show file tree

Hide file tree

Showing 2 changed files with 50 additions and 31 deletions.
diff --git a/builtin.c b/builtin.c
@@ -584,50 +584,58 @@ 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)
 {
     char *file = CvFILE(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
@@ -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();