(specpdl_unrewind): Fix corner case

* src/lisp.h (enum specbind_tag): New elem SPECPDL_NOP.

* src/eval.c (specpdl_unrewind): Zap entries that can't be applied any more,
and simplify.
(default_toplevel_binding, lexbound_p, Fbacktrace__locals): Simplify.
(do_one_unbind, mark_specpdl): Handle SPECPDL_NOP.

2 files changed, 17 insertions, 67 deletions

diff --git a/src/eval.c b/src/eval.c
index d3342289fbb..294d79e67a0 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -664,23 +664,7 @@ default_toplevel_binding (Lisp_Object symbol)
             binding = pdl;
           break;
 
-        case SPECPDL_UNWIND:
-        case SPECPDL_UNWIND_ARRAY:
-        case SPECPDL_UNWIND_PTR:
-        case SPECPDL_UNWIND_INT:
-        case SPECPDL_UNWIND_INTMAX:
-        case SPECPDL_UNWIND_EXCURSION:
-        case SPECPDL_UNWIND_VOID:
-        case SPECPDL_BACKTRACE:
-#ifdef HAVE_MODULES
-        case SPECPDL_MODULE_RUNTIME:
-        case SPECPDL_MODULE_ENVIRONMENT:
-#endif
-        case SPECPDL_LET_LOCAL:
-          break;
-
-        default:
-          emacs_abort ();
+        default: break;
         }
     }
   return binding;
@@ -707,23 +691,7 @@ lexbound_p (Lisp_Object symbol)
             }
           break;
 
-        case SPECPDL_UNWIND:
-        case SPECPDL_UNWIND_ARRAY:
-        case SPECPDL_UNWIND_PTR:
-        case SPECPDL_UNWIND_INT:
-        case SPECPDL_UNWIND_INTMAX:
-        case SPECPDL_UNWIND_EXCURSION:
-        case SPECPDL_UNWIND_VOID:
-        case SPECPDL_BACKTRACE:
-#ifdef HAVE_MODULES
-        case SPECPDL_MODULE_RUNTIME:
-        case SPECPDL_MODULE_ENVIRONMENT:
-#endif
-        case SPECPDL_LET_LOCAL:
-          break;
-
-        default:
-          emacs_abort ();
+        default: break;
         }
     }
   return false;
@@ -3721,6 +3689,7 @@ do_one_unbind (union specbinding *this_binding, bool unwinding,
                               this_binding->unwind_excursion.window);
       break;
     case SPECPDL_BACKTRACE:
+    case SPECPDL_NOP:
       break;
 #ifdef HAVE_MODULES
     case SPECPDL_MODULE_RUNTIME:
@@ -4044,17 +4013,6 @@ specpdl_unrewind (union specbinding *pdl, int distance, bool vars_only)
             save_excursion_restore (marker, window);
           }
           break;
-        case SPECPDL_UNWIND_ARRAY:
-        case SPECPDL_UNWIND_PTR:
-        case SPECPDL_UNWIND_INT:
-        case SPECPDL_UNWIND_INTMAX:
-        case SPECPDL_UNWIND_VOID:
-        case SPECPDL_BACKTRACE:
-#ifdef HAVE_MODULES
-        case SPECPDL_MODULE_RUNTIME:
-        case SPECPDL_MODULE_ENVIRONMENT:
-#endif
-          break;
         case SPECPDL_LET:
           { /* If variable has a trivial value (no forwarding), we can
                just set it.  No need to check for constant symbols here,
@@ -4097,14 +4055,16 @@ specpdl_unrewind (union specbinding *pdl, int distance, bool vars_only)
                               SET_INTERNAL_THREAD_SWITCH);
               }
             else
-              /* FIXME: If the var is not local any more, we failed
-                 to swap the old and new values.  As long as the var remains
-                 non-local, this is fine, but if it ever reverts to being
-                 local we may end up using this entry "in the wrong
-                 direction".  */
-              {}
+              /* If the var is not local any more, it can't be undone nor
+                 redone, so just zap it.
+                 This is important in case the buffer re-gains a local value
+                 before we unrewind again, in which case we'd risk applying
+                 this entry in the wrong direction.  */
+              tmp->kind = SPECPDL_NOP;
           }
           break;
+
+        default: break;
         }
     }
 }
@@ -4195,22 +4155,7 @@ NFRAMES and BASE specify the activation frame to use, as in `backtrace-frame'.
             }
             break;
 
-          case SPECPDL_UNWIND:
-          case SPECPDL_UNWIND_ARRAY:
-          case SPECPDL_UNWIND_PTR:
-          case SPECPDL_UNWIND_INT:
-          case SPECPDL_UNWIND_INTMAX:
-          case SPECPDL_UNWIND_EXCURSION:
-          case SPECPDL_UNWIND_VOID:
-          case SPECPDL_BACKTRACE:
-#ifdef HAVE_MODULES
-          case SPECPDL_MODULE_RUNTIME:
-          case SPECPDL_MODULE_ENVIRONMENT:
-#endif
-            break;
-
-          default:
-            emacs_abort ();
+          default: break;
           }
       }
   }
@@ -4274,8 +4219,12 @@ mark_specpdl (union specbinding *first, union specbinding *ptr)
         case SPECPDL_UNWIND_INT:
         case SPECPDL_UNWIND_INTMAX:
         case SPECPDL_UNWIND_VOID:
+        case SPECPDL_NOP:
           break;
 
+        /* While other loops that scan the specpdl use "default: break;"
+           for simplicity, here we explicitly list all cases and abort
+           if we find an unexpected value, as a sanity check.  */
         default:
           emacs_abort ();
         }
diff --git a/src/lisp.h b/src/lisp.h
index 19788ef07cc..deeca9bc86b 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -3284,6 +3284,7 @@ enum specbind_tag {
   SPECPDL_UNWIND_EXCURSION,     /* Likewise, on an excursion.  */
   SPECPDL_UNWIND_VOID,          /* Likewise, with no arg.  */
   SPECPDL_BACKTRACE,            /* An element of the backtrace.  */
+  SPECPDL_NOP,                  /* A filler.  */
 #ifdef HAVE_MODULES
   SPECPDL_MODULE_RUNTIME,       /* A live module runtime.  */
   SPECPDL_MODULE_ENVIRONMENT,   /* A live module environment.  */