bug#23957: [PATCH] Make fboundp an alias for symbol-function

bug#23957: [PATCH] Make fboundp an alias for symbol-function

[Robert Cochran]

[-- Attachment #1: Type: text/plain, Size: 1405 bytes --]

Hello bug-gnu-emacs,

I was looking through the C portions of the Emacs source for fun, and
noticed a FIXME. I've wanted to start hacking on Emacs, the C portions
in specific, so I jumped right in.

My patch, attached, does the following:

* The function definition in src/data.c for fboundp has been removed,
  along with the corresponding defsubr in syms_of_data, as suggested in
  the FIXME

* All places where Ffboundp is used in the C code has been replaced with
  Fsymbol_function

* A defalias form that makes fboundp an alias of symbol-function has
  been inserted into lisp/subr.el

* In lisp/emacs-lisp/byte-run.el, and also lisp/loadup.el before loading
  lisp/subr.el, all instances of fboundp have been replaced with
  symbol-function

* All other instances of fboundp have been left alone

For anyone interested, I have a copy of the Emacs repository with the
patch applied here:

https://gitlab.com/RobertCochran/emacs/tree/alias-fboundp

I understand that the FSF requires a copyright assignment for
non-trivial patches made to Emacs, and this is likely considered
non-trivial. I am perfectly happy to do so, but require guidance on how
to accomplish it. On that topic, how can I go about this process so that
I automatically perform the appropriate assignments for each
contribution?

Please do offer suggestions and improvements for any deficiency in my
patch.

Thanks,
~Robert Cochran


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: fboundp alias patch --]
[-- Type: text/x-patch, Size: 16046 bytes --]

From 0e3a73b1821c154bd20121c0d903109226f5fe7b Mon Sep 17 00:00:00 2001
From: Robert Cochran <robert-git@cochranmail.com>
Date: Sun, 10 Jul 2016 20:28:17 -0700
Subject: [PATCH] Make fboundp an alias for symbol-function

fboundp is similar enough to symbol-function that they are reasonably
interchangeable. Following the advice of the FIXME by fboundp's
definition, it has been removed as an independent function and is now
defined as an alias of symbol-function in lisp/subr.el. In the C code,
Ffboundp has been replaced with Fsymbol_function. In
lisp/emacs-lisp/byte-run.el and lisp/loadup.el, instances of fboundp
used before loading lisp/subr.el have been replaced with
symbol-function. All other uses have been unchanged, as fboundp gains a
definition after loading lisp/subr.el.
* lisp/emacs-lisp/byte-run.el (defmacro): Use symbol-function instead of
  fboundp.
* lisp/loadup.el: Use symbol-function instead of fboundp when adding to
  the load-path during bootstrap.
* lisp/subr.el: Make fboundp an alias for symbol-function.
* src/data.c (fboundp): Remove.
  (syms_of_data): Remove defsubr for fboundp.
* src/coding.c (find-operation-coding-system):
* src/composite.c (run_composition_function):
* src/doc.c (Snarf-documentation):
* src/fileio.c (substitute-in-file-name, insert-file-contents,
  choose_write_coding_system):
* src/fns.c (secure_hash):
* src/gtkutil.c (update_frame_tool_bar):
* src/keyboard.c (command_loop_1, access_keymap_keyremap):
* src/lisp.h (maybe_gc):
* src/lread.c (load, readevalloop):
* src/minibuf.c (read_minibuf, get_minibuffer):
* src/process.c (finish_after_tls_connection):
* src/syntax.c (scan_words):
* src/xdisp.c (message_dolog):
* src/xfaces.c (tty_lookup_color, defined_color): Use Fsymbol_function
  instead of Ffboundp.
---
 lisp/emacs-lisp/byte-run.el |  6 +++---
 lisp/loadup.el              |  2 +-
 lisp/subr.el                |  1 +
 src/coding.c                |  2 +-
 src/composite.c             |  2 +-
 src/data.c                  | 10 ----------
 src/doc.c                   |  2 +-
 src/fileio.c                |  8 ++++----
 src/fns.c                   |  2 +-
 src/gtkutil.c               |  2 +-
 src/keyboard.c              |  4 ++--
 src/lisp.h                  |  2 +-
 src/lread.c                 |  6 +++---
 src/minibuf.c               |  4 ++--
 src/process.c               |  2 +-
 src/syntax.c                |  4 ++--
 src/xdisp.c                 |  2 +-
 src/xfaces.c                |  6 +++---
 18 files changed, 29 insertions(+), 38 deletions(-)

diff --git a/lisp/emacs-lisp/byte-run.el b/lisp/emacs-lisp/byte-run.el
index 818c268..1abfa43 100644
--- a/lisp/emacs-lisp/byte-run.el
+++ b/lisp/emacs-lisp/byte-run.el
@@ -212,9 +212,9 @@ The return value is undefined.
 	   (if (and
 		;; If lisp-mode hasn't been loaded, there's no reason
 		;; to flush.
-		(fboundp 'lisp--el-font-lock-flush-elisp-buffers)
-		(or (not (fboundp name)) ;; new macro
-		    (and (fboundp name)  ;; existing macro
+		(symbol-function 'lisp--el-font-lock-flush-elisp-buffers)
+		(or (not (symbol-function name)) ;; new macro
+		    (and (symbol-function name)  ;; existing macro
 			 (member `(function-put ',name 'no-font-lock-keyword
 						',(get name 'no-font-lock-keyword))
 				 declarations))))
diff --git a/lisp/loadup.el b/lisp/loadup.el
index 5c16464..bb38669 100644
--- a/lisp/loadup.el
+++ b/lisp/loadup.el
@@ -61,7 +61,7 @@
 	;; FIXME this is irritatingly fragile.
 	(equal (nth 4 command-line-args) "unidata-gen.el")
 	(equal (nth 7 command-line-args) "unidata-gen-files")
-	(if (fboundp 'dump-emacs)
+	(if (symbol-function 'dump-emacs)
 	    (string-match "src/bootstrap-emacs" (nth 0 command-line-args))
 	  t))
     (let ((dir (car load-path)))
diff --git a/lisp/subr.el b/lisp/subr.el
index cf84d8b..edffd00 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -68,6 +68,7 @@ For more information, see Info node `(elisp)Declaring Functions'."
 
 (defalias 'not 'null)
 (defalias 'sxhash 'sxhash-equal)
+(defalias 'fboundp 'symbol-function)
 
 (defmacro noreturn (form)
   "Evaluate FORM, expecting it not to return.
diff --git a/src/coding.c b/src/coding.c
index 29c90f0..abf5eda 100644
--- a/src/coding.c
+++ b/src/coding.c
@@ -9877,7 +9877,7 @@ usage: (find-operation-coding-system OPERATION ARGUMENTS...)  */)
 	    return Qnil;
 	  if (! NILP (Fcoding_system_p (val)))
 	    return Fcons (val, val);
-	  if (! NILP (Ffboundp (val)))
+	  if (! NILP (Fsymbol_function (val)))
 	    {
 	      /* We use call1 rather than safe_call1
 		 so as to get bug reports about functions called here
diff --git a/src/composite.c b/src/composite.c
index 8aa6974..4e19e06 100644
--- a/src/composite.c
+++ b/src/composite.c
@@ -473,7 +473,7 @@ run_composition_function (ptrdiff_t from, ptrdiff_t to, Lisp_Object prop)
       && find_composition (to, -1, &start, &end, &prop, Qnil)
       && !composition_valid_p (start, end, prop))
     to = end;
-  if (!NILP (Ffboundp (func)))
+  if (!NILP (Fsymbol_function (func)))
     call2 (func, make_number (from), make_number (to));
 }
 
diff --git a/src/data.c b/src/data.c
index 71da916..c7e0f7e 100644
--- a/src/data.c
+++ b/src/data.c
@@ -629,15 +629,6 @@ global value outside of any lexical scope.  */)
   return (EQ (valcontents, Qunbound) ? Qnil : Qt);
 }
 
-/* FIXME: Make it an alias for function-symbol!  */
-DEFUN ("fboundp", Ffboundp, Sfboundp, 1, 1, 0,
-       doc: /* Return t if SYMBOL's function definition is not void.  */)
-  (register Lisp_Object symbol)
-{
-  CHECK_SYMBOL (symbol);
-  return NILP (XSYMBOL (symbol)->function) ? Qnil : Qt;
-}
-
 DEFUN ("makunbound", Fmakunbound, Smakunbound, 1, 1, 0,
        doc: /* Make SYMBOL's value be void.
 Return SYMBOL.  */)
@@ -3654,7 +3645,6 @@ syms_of_data (void)
   defsubr (&Smakunbound);
   defsubr (&Sfmakunbound);
   defsubr (&Sboundp);
-  defsubr (&Sfboundp);
   defsubr (&Sfset);
   defsubr (&Sdefalias);
   defsubr (&Ssetplist);
diff --git a/src/doc.c b/src/doc.c
index 6ffdad1..4965b76 100644
--- a/src/doc.c
+++ b/src/doc.c
@@ -651,7 +651,7 @@ the same file name is found in the `doc-directory'.  */)
 	      /* Attach a docstring to a function?  */
 	      else if (p[1] == 'F')
                 {
-                  if (!NILP (Ffboundp (sym)))
+                  if (!NILP (Fsymbol_function (sym)))
                     store_function_docstring (sym, pos + end + 1 - buf);
                 }
 	      else if (p[1] == 'S')
diff --git a/src/fileio.c b/src/fileio.c
index b1f9d3c..1498882 100644
--- a/src/fileio.c
+++ b/src/fileio.c
@@ -1718,7 +1718,7 @@ those `/' is discarded.  */)
 
   /* See if any variables are substituted into the string.  */
 
-  if (!NILP (Ffboundp (Qsubstitute_env_in_file_name)))
+  if (!NILP (Fsymbol_function (Qsubstitute_env_in_file_name)))
     {
       Lisp_Object name
 	= (!substituted ? filename
@@ -4358,7 +4358,7 @@ by calling `format-decode', which see.  */)
   if (set_coding_system)
     Vlast_coding_system_used = coding_system;
 
-  if (! NILP (Ffboundp (Qafter_insert_file_set_coding)))
+  if (! NILP (Fsymbol_function (Qafter_insert_file_set_coding)))
     {
       insval = call2 (Qafter_insert_file_set_coding, make_number (inserted),
 		      visit);
@@ -4551,7 +4551,7 @@ choose_write_coding_system (Lisp_Object start, Lisp_Object end, Lisp_Object file
     {
       val = Vcoding_system_for_write;
       if (coding_system_require_warning
-	  && !NILP (Ffboundp (Vselect_safe_coding_system_function)))
+	  && !NILP (Fsymbol_function (Vselect_safe_coding_system_function)))
 	/* Confirm that VAL can surely encode the current region.  */
 	val = call5 (Vselect_safe_coding_system_function,
 		     start, end, list2 (Qt, val),
@@ -4610,7 +4610,7 @@ choose_write_coding_system (Lisp_Object start, Lisp_Object end, Lisp_Object file
 	}
 
       if (!force_raw_text
-	  && !NILP (Ffboundp (Vselect_safe_coding_system_function)))
+	  && !NILP (Fsymbol_function (Vselect_safe_coding_system_function)))
 	{
 	  /* Confirm that VAL can surely encode the current region.  */
 	  val = call5 (Vselect_safe_coding_system_function,
diff --git a/src/fns.c b/src/fns.c
index 270dfb4..7a34d95 100644
--- a/src/fns.c
+++ b/src/fns.c
@@ -4917,7 +4917,7 @@ secure_hash (Lisp_Object algorithm, Lisp_Object object, Lisp_Object start,
 		}
 
 	      if (!force_raw_text
-		  && !NILP (Ffboundp (Vselect_safe_coding_system_function)))
+		  && !NILP (Fsymbol_function (Vselect_safe_coding_system_function)))
 		/* Confirm that VAL can surely encode the current region.  */
 		coding_system = call4 (Vselect_safe_coding_system_function,
 				       make_number (b), make_number (e),
diff --git a/src/gtkutil.c b/src/gtkutil.c
index 88e6d30..6b7d187 100644
--- a/src/gtkutil.c
+++ b/src/gtkutil.c
@@ -4818,7 +4818,7 @@ update_frame_tool_bar (struct frame *f)
         }
 
       specified_file = file_for_image (image);
-      if (!NILP (specified_file) && !NILP (Ffboundp (Qx_gtk_map_stock)))
+      if (!NILP (specified_file) && !NILP (Fsymbol_function (Qx_gtk_map_stock)))
         stock = call1 (Qx_gtk_map_stock, specified_file);
 
       if (CONSP (stock))
diff --git a/src/keyboard.c b/src/keyboard.c
index 653f527..b87bd83 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -1353,7 +1353,7 @@ command_loop_1 (void)
       /* If it has changed current-menubar from previous value,
 	 really recompute the menubar from the value.  */
       if (! NILP (Vlucid_menu_bar_dirty_flag)
-	  && !NILP (Ffboundp (Qrecompute_lucid_menubar)))
+	  && !NILP (Fsymbol_function (Qrecompute_lucid_menubar)))
 	call0 (Qrecompute_lucid_menubar);
 
       Vthis_command = Qnil;
@@ -8697,7 +8697,7 @@ access_keymap_keyremap (Lisp_Object map, Lisp_Object key, Lisp_Object prompt,
 
   /* Handle a symbol whose function definition is a keymap
      or an array.  */
-  if (SYMBOLP (next) && !NILP (Ffboundp (next))
+  if (SYMBOLP (next) && !NILP (Fsymbol_function (next))
       && (ARRAYP (XSYMBOL (next)->function)
 	  || KEYMAPP (XSYMBOL (next)->function)))
     next = Fautoload_do_load (XSYMBOL (next)->function, next, Qnil);
diff --git a/src/lisp.h b/src/lisp.h
index e0eb52a..fba56a4 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -4680,7 +4680,7 @@ maybe_gc (void)
 INLINE bool
 functionp (Lisp_Object object)
 {
-  if (SYMBOLP (object) && !NILP (Ffboundp (object)))
+  if (SYMBOLP (object) && !NILP (Fsymbol_function (object)))
     {
       object = Findirect_function (object, Qt);
 
diff --git a/src/lread.c b/src/lread.c
index ecd4827..6cd3b5a 100644
--- a/src/lread.c
+++ b/src/lread.c
@@ -1344,7 +1344,7 @@ Return t if the file exists and loads successfully.  */)
   unbind_to (count, Qnil);
 
   /* Run any eval-after-load forms for this file.  */
-  if (!NILP (Ffboundp (Qdo_after_load_evaluation)))
+  if (!NILP (Fsymbol_function (Qdo_after_load_evaluation)))
     call1 (Qdo_after_load_evaluation, hist_file_name) ;
 
   xfree (saved_doc_string);
@@ -1787,7 +1787,7 @@ readevalloop (Lisp_Object readcharfun,
   bool first_sexp = 1;
   Lisp_Object macroexpand = intern ("internal-macroexpand-for-load");
 
-  if (NILP (Ffboundp (macroexpand))
+  if (NILP (Fsymbol_function (macroexpand))
       /* Don't macroexpand in .elc files, since it should have been done
 	 already.  We actually don't know whether we're in a .elc file or not,
 	 so we use circumstantial evidence: .el files normally go through
@@ -1828,7 +1828,7 @@ readevalloop (Lisp_Object readcharfun,
   /* Try to ensure sourcename is a truename, except whilst preloading.  */
   if (NILP (Vpurify_flag)
       && !NILP (sourcename) && !NILP (Ffile_name_absolute_p (sourcename))
-      && !NILP (Ffboundp (Qfile_truename)))
+      && !NILP (Fsymbol_function (Qfile_truename)))
     sourcename = call1 (Qfile_truename, sourcename) ;
 
   LOADHIST_ATTACH (sourcename);
diff --git a/src/minibuf.c b/src/minibuf.c
index 57eea05..ee6b435 100644
--- a/src/minibuf.c
+++ b/src/minibuf.c
@@ -672,7 +672,7 @@ read_minibuf (Lisp_Object map, Lisp_Object initial, Lisp_Object prompt,
   bset_keymap (current_buffer, map);
 
   /* Turn on an input method stored in INPUT_METHOD if any.  */
-  if (STRINGP (input_method) && !NILP (Ffboundp (Qactivate_input_method)))
+  if (STRINGP (input_method) && !NILP (Fsymbol_function (Qactivate_input_method)))
     call1 (Qactivate_input_method, input_method);
 
   run_hook (Qminibuffer_setup_hook);
@@ -801,7 +801,7 @@ get_minibuffer (EMACS_INT depth)
       reset_buffer (XBUFFER (buf));
       record_unwind_current_buffer ();
       Fset_buffer (buf);
-      if (!NILP (Ffboundp (intern ("minibuffer-inactive-mode"))))
+      if (!NILP (Fsymbol_function (intern ("minibuffer-inactive-mode"))))
 	call0 (intern ("minibuffer-inactive-mode"));
       else
         Fkill_all_local_variables ();
diff --git a/src/process.c b/src/process.c
index bdbdefa..4189e45 100644
--- a/src/process.c
+++ b/src/process.c
@@ -3088,7 +3088,7 @@ finish_after_tls_connection (Lisp_Object proc)
   Lisp_Object contact = p->childp;
   Lisp_Object result = Qt;
 
-  if (!NILP (Ffboundp (Qnsm_verify_connection)))
+  if (!NILP (Fsymbol_function (Qnsm_verify_connection)))
     result = call3 (Qnsm_verify_connection,
 		    proc,
 		    Fplist_get (contact, QChost),
diff --git a/src/syntax.c b/src/syntax.c
index f8d987b..260ea92 100644
--- a/src/syntax.c
+++ b/src/syntax.c
@@ -1455,7 +1455,7 @@ scan_words (register ptrdiff_t from, register EMACS_INT count)
       /* Now CH0 is a character which begins a word and FROM is the
          position of the next character.  */
       func = CHAR_TABLE_REF (Vfind_word_boundary_function_table, ch0);
-      if (! NILP (Ffboundp (func)))
+      if (! NILP (Fsymbol_function (func)))
 	{
 	  pos = call2 (func, make_number (from - 1), make_number (end));
 	  if (INTEGERP (pos) && from < XINT (pos) && XINT (pos) <= ZV)
@@ -1505,7 +1505,7 @@ scan_words (register ptrdiff_t from, register EMACS_INT count)
       /* Now CH1 is a character which ends a word and FROM is the
          position of it.  */
       func = CHAR_TABLE_REF (Vfind_word_boundary_function_table, ch1);
-      if (! NILP (Ffboundp (func)))
+      if (! NILP (Fsymbol_function (func)))
  	{
 	  pos = call2 (func, make_number (from), make_number (beg));
 	  if (INTEGERP (pos) && BEGV <= XINT (pos) && XINT (pos) < from)
diff --git a/src/xdisp.c b/src/xdisp.c
index 14d6f8f..b6e8cf8 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -10116,7 +10116,7 @@ message_dolog (const char *m, ptrdiff_t nbytes, bool nlflag, bool multibyte)
       bool newbuffer = NILP (Fget_buffer (Vmessages_buffer_name));
       Fset_buffer (Fget_buffer_create (Vmessages_buffer_name));
       if (newbuffer
-	  && !NILP (Ffboundp (intern ("messages-buffer-mode"))))
+	  && !NILP (Fsymbol_function (intern ("messages-buffer-mode"))))
 	call0 (intern ("messages-buffer-mode"));
 
       bset_undo_list (current_buffer, Qt);
diff --git a/src/xfaces.c b/src/xfaces.c
index 0a1315d..145c8ea 100644
--- a/src/xfaces.c
+++ b/src/xfaces.c
@@ -844,7 +844,7 @@ tty_lookup_color (struct frame *f, Lisp_Object color, XColor *tty_color,
 {
   Lisp_Object frame, color_desc;
 
-  if (!STRINGP (color) || NILP (Ffboundp (Qtty_color_desc)))
+  if (!STRINGP (color) || NILP (Fsymbol_function (Qtty_color_desc)))
     return false;
 
   XSETFRAME (frame, f);
@@ -875,7 +875,7 @@ tty_lookup_color (struct frame *f, Lisp_Object color, XColor *tty_color,
 	     a standard color, we just give up and use TTY_COLOR.  */
 	  if ((!STRINGP (XCAR (color_desc))
 	       || NILP (Fstring_equal (color, XCAR (color_desc))))
-	      && !NILP (Ffboundp (Qtty_color_standard_values)))
+	      && !NILP (Fsymbol_function (Qtty_color_standard_values)))
 	    {
 	      /* Look up STD_COLOR separately.  */
 	      rgb = call1 (Qtty_color_standard_values, color);
@@ -964,7 +964,7 @@ defined_color (struct frame *f, const char *color_name, XColor *color_def,
 Lisp_Object
 tty_color_name (struct frame *f, int idx)
 {
-  if (idx >= 0 && !NILP (Ffboundp (Qtty_color_by_index)))
+  if (idx >= 0 && !NILP (Fsymbol_function (Qtty_color_by_index)))
     {
       Lisp_Object frame;
       Lisp_Object coldesc;
-- 
2.7.4

bug#23957: [PATCH] Make fboundp an alias for symbol-function

[Drew Adams]

Why would we do this?

It is not backward compatible, since `fboundp' returns t if
there is a function value and `symbol-function' returns that
function value.  This will break any code that depends on
getting t or nil.

What is the point of this "fix"?

bug#23957: [PATCH] Make fboundp an alias for symbol-function

[Philipp Stephani]

[-- Attachment #1: Type: text/plain, Size: 456 bytes --]

Drew Adams <drew.adams@oracle.com> schrieb am Di., 12. Juli 2016 um
18:14 Uhr:

> It is not backward compatible, since `fboundp' returns t if
> there is a function value and `symbol-function' returns that
> function value.  This will break any code that depends on
> getting t or nil.
>

And the docstring of `fboundp' does specify that it returns t (not just
non-nil), so this is indeed a breaking chance.

I'd suggest to simply remove the FIXME instead.

[-- Attachment #2: Type: text/html, Size: 783 bytes --]

bug#23957: [PATCH] Make fboundp an alias for symbol-function

[Drew Adams]

> And the docstring of `fboundp' does specify that it returns t
> (not just non-nil), so this is indeed a breaking chance.

Yes, I meant to add that.  Thx.

> I'd suggest to simply remove the FIXME instead.

I'd be somewhat curious about who wrote it and why.  Maybe
there is something else that could be done to satisfy whatever
the perceived need was.

bug#23957: [PATCH] Make fboundp an alias for symbol-function

[Robert Cochran]

I didn't put too much thought into reading the FIXME; I figured that
someone smarter than myself knew what they were doing when requesting
the change.

I'd personally argue that anyone making an explicit check for t, or
anything that particularly needs t rather than any true value is just
asking for lossage, but I can see why people would disagree with that
assertion.

FWIW, In every placed I changed occurrences of fboundp to
symbol-function, both in Lisp and C, used only the truthiness of the
return rather than explicitly checking for t.

I also ran the test suite with and without my patch applied, and noticed
no difference in the number of failing tests.

Anyways, I'm willing to toss this patch and do something else if that is
the general consensus.

Thanks,
~Robert Cochran

bug#23957: [PATCH] Make fboundp an alias for symbol-function

[Drew Adams]

> I didn't put too much thought into reading the FIXME;

Nor did I. ;-)

> I figured that someone smarter than myself knew what they were
> doing when requesting the change.

Yes, no doubt s?he had a good reason.  Or at least a reason.
But apparently it was not recorded, so we can only wonder or
guess.

My guess is that the person just figured that, like `member',
people can use `symbol-function' as a general Boolean - which
is true.  But `fboundp' has been around forever, and there is
no telling what code might expect its value to be either `t'
or `nil'.  And if someone really wants to use `symbol-function'
to either get the function or test whether there is one, s?he
can already do that.

> I'd personally argue that anyone making an explicit check for t, or
> anything that particularly needs t rather than any true value is just
> asking for lossage, but I can see why people would disagree with that
> assertion.

It doesn't matter what we might think of such a check.  The point
is that such checks might exist, and there is really no good
reason (that I can see) for breaking such code.  Again: anyone
can already use `symbol-function' to get the desired effect, and
its name speaks much better to the combined behavior desired in
that case.

> FWIW, In every placed I changed occurrences of fboundp to
> symbol-function, both in Lisp and C, used only the truthiness
> of the return rather than explicitly checking for t.

That's irrelevant (IMO).  The code that GNU distributes with
Emacs is but a small part of the Emacs-Lisp code that is out
there.

> I also ran the test suite with and without my patch applied,
> and noticed no difference in the number of failing tests.

Again - you were testing in the tiny GNU Emacs distributed-code
sandbox.  The Emacs world is a much bigger box.

> Anyways, I'm willing to toss this patch and do something else
> if that is the general consensus.

I can't speak for the consensus, but that would be my hope.  And
thanks for pitching in!  Sorry to seem so critical of a first foray
into helping.

bug#23957: [PATCH] Make fboundp an alias for symbol-function

[Robert Cochran]

Drew Adams <drew.adams@oracle.com> writes:

>> I'd personally argue that anyone making an explicit check for t, or
>> anything that particularly needs t rather than any true value is just
>> asking for lossage, but I can see why people would disagree with that
>> assertion.
>
> It doesn't matter what we might think of such a check.  The point
> is that such checks might exist, and there is really no good
> reason (that I can see) for breaking such code.  Again: anyone
> can already use `symbol-function' to get the desired effect, and
> its name speaks much better to the combined behavior desired in
> that case.
>> FWIW, In every placed I changed occurrences of fboundp to
>> symbol-function, both in Lisp and C, used only the truthiness
>> of the return rather than explicitly checking for t.
>
> That's irrelevant (IMO).  The code that GNU distributes with
> Emacs is but a small part of the Emacs-Lisp code that is out
> there.
>
>> I also ran the test suite with and without my patch applied,
>> and noticed no difference in the number of failing tests.
>
> Again - you were testing in the tiny GNU Emacs distributed-code
> sandbox.  The Emacs world is a much bigger box.

Fair enough. I was never going to cast a net big enough to hit
everything. I agree with Stefan that most functions will be behave fine
in the face of the change, but given his hesitation, I'm not going to
push it. You guys know better than I do.

>> Anyways, I'm willing to toss this patch and do something else
>> if that is the general consensus.
>
> I can't speak for the consensus, but that would be my hope.  And
> thanks for pitching in!  Sorry to seem so critical of a first foray
> into helping.

Nah, don't worry about. I was expecting /something/ to snag my
patch. You clearly expressed your concerns about my changes, and we were
able to discuss and be civil. Best I can ask for, really. In the end,
I'll end up doing a cleanup of some sort, even if all that is is
removing the FIXME.

I've been hearing about how there are less and less people that
understand the C core of Emacs, so I'm aiming to help by becoming one of
those people. Barring any extreme levels of inter-personal friction in
getting things accomplished, I'm hopefully just getting started with
this.

Thanks,
~Robert Cochran

bug#23957: [PATCH] Make fboundp an alias for symbol-function

[Drew Adams]

> I've been hearing about how there are less and less people that
> understand the C core of Emacs, so I'm aiming to help by becoming one of
> those people. Barring any extreme levels of inter-personal friction in
> getting things accomplished, I'm hopefully just getting started with
> this.

That is excellent.  There are some really knowledgeable and
experienced Emacs C developers here (who are likewise for Emacs
Lisp development).  I expect it would be a great project and a
great way to learn, to help with this.  There are also others
who, like yourself, are tackling this fresh.  Thank you.

bug#23957: [PATCH] Make fboundp an alias for symbol-function

[Sora Firestorm]

Drew Adams <drew.adams@oracle.com> writes:

>> I'd suggest to simply remove the FIXME instead.
>
> I'd be somewhat curious about who wrote it and why.  Maybe
> there is something else that could be done to satisfy whatever
> the perceived need was.

I just did a blame for that line, and it lead back to commit eadf1faa
("Conflate Qnil and Qunbound for `symbol-function'.") by Stefan Monnier
<monnier@iro.umontreal.ca> (CC'd).

bug#23957: [PATCH] Make fboundp an alias for symbol-function

[Drew Adams]

> >> I'd suggest to simply remove the FIXME instead.
> >
> > I'd be somewhat curious about who wrote it and why.  Maybe
> > there is something else that could be done to satisfy whatever
> > the perceived need was.
> 
> I just did a blame for that line, and it lead back to commit eadf1faa
> ("Conflate Qnil and Qunbound for `symbol-function'.") by Stefan Monnier
> <monnier@iro.umontreal.ca> (CC'd).

Yes, well, perhaps Stefan will chime in with the reason(s) for it.

But to my mind the simple action "Conflate Qnil and Qunbound for
`symbol-function'" sounds like it is about doing something to
`symbol-function' rather than doing something to `fboundp'.

It is the behavior (hence meaning) of `fboundp' that changes
if you set it as an alias for `symbol-function'.  The latter's
behavior does not change.

bug#23957: [PATCH] Make fboundp an alias for symbol-function

[Stefan Monnier]

> And the docstring of `fboundp' does specify that it returns t (not just
> non-nil), so this is indeed a breaking chance.
> I'd suggest to simply remove the FIXME instead.

FWIW, the reason why I put a FIXME instead of making this change, is
that I wasn't sure indeed exactly how to make such a change.

I expect 99% of the uses of fboundp don't care about the distinction
between t and other non-nil values.  And at least 90% of the remaining
1% is probably ill-advised to rely on this distinction.  But the benefit
of redefining fboundp as an alias rather than as its own function is
probably too small to justify risking such breakage.


        Stefan

bug#23957: [PATCH] Make fboundp an alias for symbol-function

[Robert Cochran]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> I expect 99% of the uses of fboundp don't care about the distinction
> between t and other non-nil values.  And at least 90% of the remaining
> 1% is probably ill-advised to rely on this distinction.  But the benefit
> of redefining fboundp as an alias rather than as its own function is
> probably too small to justify risking such breakage.

So then, is it generally agreed upon that I submit a new patch that
merely strips the FIXME, or does this need more discussion? Trying to
make sure I'm not jumping the gun by confirming before I do so.

Thanks,
~Robert Cochran

bug#23957: [PATCH] Make fboundp an alias for symbol-function

[Robert Cochran]

Submitted a patch to remove the FIXME as Bug#23988

~Robert Cochran