unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#23486: 25.0.93; Modules: features missing from make_function
@ 2016-05-09 16:37 Philipp Stephani
  2016-09-11 14:13 ` Philipp Stephani
                   ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: Philipp Stephani @ 2016-05-09 16:37 UTC (permalink / raw)
  To: 23486


emacs_env::make_function lacks the following features supported by
`defun':

1. Functions with both optional and rest arguments.
2. Specification of parameter names.
3. Integration with `help-function-arglist'.
4. Specification of interactive forms.
5. Specification of declare forms.
6. Docstrings containing null or non-Unicode characters.

(6) is probably rather unimportant.  (5) is probably not implementable
(would require wrapping `defun', not `lambda').  (1)–(4) are more severe
and quite limit the usefulness of make_function right now; for a
truly generic `defun'-like construct one currently has to eval a `defun'
form wrapping another function.

To solve (1)–(3), I'd propose replacing the "arity" arguments with a
true arglist specification.  This could either be at the C level, e.g.

    ptrdiff_t num_mandatory_args, char** mandatory_arg_names,
    ptrdiff_t num_optional_args, char** optional_arg_names,
    char* rest_arg_name

or by requiring to pass a Lisp argument list.

To solve (4) I'd propose to pass another value for the interactive form,
probably as emacs_value* (to support non-interactive functions).

As an alternative, if people feel this would require too many
parameters, I'd propose reverting the change that adds the documentation
string.  A docstring without arglist is not very useful.  We could also
remove the arity parameters and have the C function check the arity
itself.



In GNU Emacs 25.0.93.5 (x86_64-unknown-linux-gnu, GTK+ Version 3.10.8)
 of 2016-04-24 built on localhost
Repository revision: 0cd2e923dba8d8c7128b0c084ce6af22069e8db5
Windowing system distributor 'The X.Org Foundation', version 11.0.11501000
System Description:	Ubuntu 14.04 LTS

Configured using:
 'configure --with-modules'

Configured features:
XPM JPEG TIFF GIF PNG SOUND GSETTINGS NOTIFY FREETYPE XFT ZLIB
TOOLKIT_SCROLL_BARS GTK3 X11 MODULES

Important settings:
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message dired format-spec rfc822 mml
mml-sec password-cache epg epg-config gnus-util mm-decode mm-bodies
mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader sendmail
rfc2047 rfc2045 ietf-drums mm-util help-fns help-mode easymenu
cl-loaddefs pcase cl-lib mail-prsvr mail-utils time-date mule-util
tooltip eldoc electric uniquify ediff-hook vc-hooks lisp-float-type
mwheel x-win term/common-win x-dnd tool-bar dnd fontset image regexp-opt
fringe tabulated-list newcomment elisp-mode lisp-mode prog-mode register
page menu-bar rfn-eshadow timer select scroll-bar mouse jit-lock
font-lock syntax facemenu font-core frame cl-generic cham georgian
utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao korean
japanese eucjp-ms cp51932 hebrew greek romanian slovak czech european
ethiopic indian cyrillic chinese charscript case-table epa-hook
jka-cmpr-hook help simple abbrev minibuffer cl-preloaded nadvice
loaddefs button faces cus-face macroexp files text-properties overlay
sha1 md5 base64 format env code-pages mule custom widget
hashtable-print-readable backquote inotify dynamic-setting
system-font-setting font-render-setting move-toolbar gtk x-toolkit x
multi-tty make-network-process emacs)

Memory information:
((conses 16 88004 8077)
 (symbols 48 19855 0)
 (miscs 40 325 197)
 (strings 32 14723 4466)
 (string-bytes 1 436825)
 (vectors 16 12084)
 (vector-slots 8 438946 3946)
 (floats 8 164 12)
 (intervals 56 202 0)
 (buffers 976 12)
 (heap 1024 36386 931))

-- 
Google Germany GmbH
Erika-Mann-Straße 33
80636 München

Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Geschäftsführer: Matthew Scott Sucherman, Paul Terence Manicle

Diese E-Mail ist vertraulich.  Wenn Sie nicht der richtige Adressat sind,
leiten Sie diese bitte nicht weiter, informieren Sie den Absender und löschen
Sie die E-Mail und alle Anhänge.  Vielen Dank.

This e-mail is confidential.  If you are not the right addressee please do not
forward it, please inform the sender, and please erase this e-mail including
any attachments.  Thanks.





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#23486: 25.0.93; Modules: features missing from make_function
  2016-05-09 16:37 bug#23486: 25.0.93; Modules: features missing from make_function Philipp Stephani
@ 2016-09-11 14:13 ` Philipp Stephani
  2016-09-11 14:57 ` npostavs
  2017-03-27  3:57 ` npostavs
  2 siblings, 0 replies; 15+ messages in thread
From: Philipp Stephani @ 2016-09-11 14:13 UTC (permalink / raw)
  To: 23486


[-- Attachment #1.1: Type: text/plain, Size: 1209 bytes --]

Philipp Stephani <p.stephani2@gmail.com> schrieb am Mo., 9. Mai 2016 um
18:39 Uhr:

>
> emacs_env::make_function lacks the following features supported by
> `defun':
>
> 1. Functions with both optional and rest arguments.
> 2. Specification of parameter names.
> 3. Integration with `help-function-arglist'.
> 4. Specification of interactive forms.
> 5. Specification of declare forms.
> 6. Docstrings containing null or non-Unicode characters.
>
> (6) is probably rather unimportant.  (5) is probably not implementable
> (would require wrapping `defun', not `lambda').  (1)–(4) are more severe
> and quite limit the usefulness of make_function right now; for a
> truly generic `defun'-like construct one currently has to eval a `defun'
> form wrapping another function.
>
> To solve (1)–(3), I'd propose replacing the "arity" arguments with a
> true arglist specification.  This could either be at the C level, e.g.
>
>     ptrdiff_t num_mandatory_args, char** mandatory_arg_names,
>     ptrdiff_t num_optional_args, char** optional_arg_names,
>     char* rest_arg_name
>
> or by requiring to pass a Lisp argument list.
>

I've attached a patch for fixing (1)-(4) and (6).

[-- Attachment #1.2: Type: text/html, Size: 2034 bytes --]

[-- Attachment #2: 0001-Introduce-new-module-function-make_function_ext.patch --]
[-- Type: application/octet-stream, Size: 8154 bytes --]

From 47ff03da305ec118fb841cac4c7ac994b2eeb52c Mon Sep 17 00:00:00 2001
From: Philipp Stephani <phst@google.com>
Date: Sun, 11 Sep 2016 16:09:01 +0200
Subject: [PATCH] Introduce new module function `make_function_ext'

This function allows specifying a full argument list and all other
features of `lambda'; see Bug#23486.

* src/emacs-module.c (module_make_function_ext): New function.
(initialize_environment): Use new function.

* modules/mod-test/mod-test.c (Fmod_test_sum_2)
(emacs_module_init): Add an example module function using
make_function_ext.

* modules/mod-test/test.el (mod-test-sum-2-test): Add a test for
new example module function.
---
 modules/mod-test/mod-test.c | 29 +++++++++++++++++++++
 modules/mod-test/test.el    |  9 +++++++
 src/emacs-module.c          | 63 +++++++++++++++++++++++++++++++++++++++++++++
 src/emacs-module.h          | 22 ++++++++++++++++
 4 files changed, 123 insertions(+)

diff --git a/modules/mod-test/mod-test.c b/modules/mod-test/mod-test.c
index 3c8ab0f..1c2e5c6 100644
--- a/modules/mod-test/mod-test.c
+++ b/modules/mod-test/mod-test.c
@@ -52,6 +52,26 @@ Fmod_test_sum (emacs_env *env, ptrdiff_t nargs, emacs_value args[], void *data)
   return env->make_integer (env, r);
 }
 
+static emacs_value
+Fmod_test_sum_2 (emacs_env *env, ptrdiff_t nargs, emacs_value *args, void *data)
+{
+  intmax_t accumulator = 0;
+
+  for (ptrdiff_t i = 0; i < nargs; ++i)
+    {
+      if (! env->is_not_nil (env, args[i]))
+        continue;
+      intmax_t arg = env->extract_integer (env, args[i]);
+      if (__builtin_add_overflow (accumulator, arg, &accumulator))
+        {
+          env->non_local_exit_signal (env, env->intern (env, "overflow-error"), env->intern (env, "nil"));
+          break;
+        }
+    }
+
+  return env->make_integer (env, accumulator);
+}
+
 
 /* Signal '(error 56).  */
 static emacs_value
@@ -263,6 +283,15 @@ emacs_module_init (struct emacs_runtime *ert)
 
 #undef DEFUN
 
+  emacs_value list_args[] = {env->intern (env, "a"),
+                             env->intern (env, "&optional"), env->intern (env, "b"),
+                             env->intern (env, "&rest"), env->intern (env, "rest")};
+  bind_function (env, "mod-test-sum-2",
+                 env->make_function_ext (env,
+                                         env->funcall (env, env->intern (env, "list"), 5, list_args),
+                                         env->intern (env, "nil"), NULL,
+                                         Fmod_test_sum_2, NULL));
+
   provide (env, "mod-test");
   return 0;
 }
diff --git a/modules/mod-test/test.el b/modules/mod-test/test.el
index 2d363c3..037a996 100644
--- a/modules/mod-test/test.el
+++ b/modules/mod-test/test.el
@@ -56,6 +56,15 @@
   (should-error (mod-test-sum -1 most-negative-fixnum)
                 :type 'overflow-error))
 
+(ert-deftest mod-test-sum-2-test ()
+  (should-error (mod-test-sum-2) :type 'wrong-number-of-arguments)
+  (should (equal (mod-test-sum-2 1) 1))
+  (should (equal (mod-test-sum-2 1 2) 3))
+  (should (equal (mod-test-sum-2 1 2 3) 6))
+  (should (equal (mod-test-sum-2 1 2 3 4) 10))
+  (should-error (mod-test-sum-2 'foo) :type 'wrong-type-argument)
+  (should-error (mod-test-sum-2 most-positive-fixnum 1) :type 'overflow-error))
+
 (ert-deftest mod-test-sum-docstring ()
   (should (string= (documentation 'mod-test-sum) "Return A + B")))
 
diff --git a/src/emacs-module.c b/src/emacs-module.c
index 724d24a..25fcdd2 100644
--- a/src/emacs-module.c
+++ b/src/emacs-module.c
@@ -415,6 +415,68 @@ module_make_function (emacs_env *env, ptrdiff_t min_arity, ptrdiff_t max_arity,
 }
 
 static emacs_value
+module_make_function_ext (emacs_env *env, emacs_value arglist,
+                          emacs_value docstring, emacs_value *interactive,
+                          emacs_subr function, void *data)
+{
+  MODULE_FUNCTION_BEGIN (module_nil);
+
+  Lisp_Object arglist_obj = value_to_lisp (arglist);
+  CHECK_LIST (arglist_obj);
+
+  Lisp_Object docstring_obj = value_to_lisp (docstring);
+  if (! NILP (docstring_obj))
+    /* Explicitly check whether the docstring is indeed a string, so that
+       callers can't sneak in body forms.  */
+    CHECK_STRING (docstring_obj);
+
+  /* Build up lists that forward the arguments to internal--module-call.  */
+  Lisp_Object normal_args = Qnil;
+  Lisp_Object rest_args = Qnil;
+  for (Lisp_Object it = arglist_obj; ! NILP (it); it = CDR (it))
+    {
+      /* We don't check for invalid parameters here.  funcall_lambda will check.  */
+      Lisp_Object arg = CAR (it);
+      if (EQ (arg, Qand_optional))
+        continue;
+      if (EQ (arg, Qand_rest))
+        {
+          rest_args = Fcons (CAR (CDR (it)), rest_args);
+          break;
+        }
+      normal_args = Fcons (arg, normal_args);
+    }
+  normal_args = Fnreverse (normal_args);
+
+  /* FIXME: This should be freed when envobj is GC'd.  */
+  struct module_fun_env *envptr = xmalloc (sizeof *envptr);
+  /* The actual argument count check is done by funcall_lambda.  */
+  envptr->min_arity = 0;
+  envptr->max_arity = emacs_variadic_function;
+  envptr->subr = function;
+  envptr->data = data;
+  Lisp_Object envobj = make_save_ptr (envptr);
+
+  /* Build up the function definition.  It is a normal lambda form.  */
+  Lisp_Object ret = CALLN (Fappend,
+                           list3 (Qapply,
+                                  list2 (Qfunction, Qinternal__module_call),
+                                  envobj),
+                           normal_args, rest_args);
+  ret = list1 (ret);
+
+  if (interactive != NULL)
+    ret = Fcons (list2 (Qinteractive, value_to_lisp (*interactive)), ret);
+
+  if (! NILP (docstring_obj))
+    ret = Fcons (docstring_obj, ret);
+
+  ret = Fcons (Qlambda, Fcons (arglist_obj, ret));
+
+  return lisp_to_value (ret);
+}
+
+static emacs_value
 module_funcall (emacs_env *env, emacs_value fun, ptrdiff_t nargs,
 		emacs_value args[])
 {
@@ -916,6 +978,7 @@ initialize_environment (emacs_env *env, struct emacs_env_private *priv)
   env->non_local_exit_signal = module_non_local_exit_signal;
   env->non_local_exit_throw = module_non_local_exit_throw;
   env->make_function = module_make_function;
+  env->make_function_ext = module_make_function_ext;
   env->funcall = module_funcall;
   env->intern = module_intern;
   env->type_of = module_type_of;
diff --git a/src/emacs-module.h b/src/emacs-module.h
index ae7311b..da6a103 100644
--- a/src/emacs-module.h
+++ b/src/emacs-module.h
@@ -185,6 +185,28 @@ struct emacs_env_25
 		   emacs_value val);
 
   ptrdiff_t (*vec_size) (emacs_env *env, emacs_value vec);
+
+  /* Variant of make_function that allows specifying a full argument list,
+     documentation string, and interactive form.  This allows defining
+     functions that are impossible to define with make_function, such as
+     functions with both optional and rest arguments.  It also improves help
+     display for the function, as the arguments are named.  ARGLIST must be a
+     list specifying the function parameters, as with `lambda' or `defun'.
+     DOCSTRING must be either nil (for an undocumented function) or a string.
+     INTERACTIVE must be either NULL (for a non-interactive function) or a
+     pointer to a Lisp object to be used as argument to `interactive', which
+     see.  When calling the function, it is unspecified whether optional
+     arguments are passed as nil or not passed at all.  */
+  emacs_value (*make_function_ext) (emacs_env *env,
+                                    emacs_value arglist,
+                                    emacs_value docstring,
+                                    emacs_value *interactive,
+                                    emacs_value (*function) (emacs_env *env,
+                                                             ptrdiff_t nargs,
+                                                             emacs_value *args,
+                                                             void *data)
+                                    EMACS_NOEXCEPT,
+                                    void *data);
 };
 
 /* Every module should define a function as follows.  */
-- 
2.9.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* bug#23486: 25.0.93; Modules: features missing from make_function
  2016-05-09 16:37 bug#23486: 25.0.93; Modules: features missing from make_function Philipp Stephani
  2016-09-11 14:13 ` Philipp Stephani
@ 2016-09-11 14:57 ` npostavs
  2017-03-26 20:02   ` Philipp Stephani
  2017-03-27  3:57 ` npostavs
  2 siblings, 1 reply; 15+ messages in thread
From: npostavs @ 2016-09-11 14:57 UTC (permalink / raw)
  To: Philipp Stephani; +Cc: 23486

Philipp Stephani <p.stephani2@gmail.com> writes:

> emacs_env::make_function lacks the following features supported by
> `defun':
>
> 1. Functions with both optional and rest arguments.
> 2. Specification of parameter names.
> 3. Integration with `help-function-arglist'.
> 4. Specification of interactive forms.
> 5. Specification of declare forms.
> 6. Docstrings containing null or non-Unicode characters.
>
> (6) is probably rather unimportant.  (5) is probably not implementable
> (would require wrapping `defun', not `lambda').  (1)–(4) are more severe
> and quite limit the usefulness of make_function right now; for a
> truly generic `defun'-like construct one currently has to eval a `defun'
> form wrapping another function.

Shouldn't modules be providing a DEFUN-like construct instead?  That is,
I thought the idea of modules was to enable writing primitive
subroutines.

>
> To solve (1)–(3), I'd propose replacing the "arity" arguments with a
> true arglist specification.  This could either be at the C level, e.g.
>
>     ptrdiff_t num_mandatory_args, char** mandatory_arg_names,
>     ptrdiff_t num_optional_args, char** optional_arg_names,
>     char* rest_arg_name
>
> or by requiring to pass a Lisp argument list.
>
> To solve (4) I'd propose to pass another value for the interactive form,
> probably as emacs_value* (to support non-interactive functions).
>
> As an alternative, if people feel this would require too many
> parameters, I'd propose reverting the change that adds the documentation
> string.  A docstring without arglist is not very useful.  We could also
> remove the arity parameters and have the C function check the arity
> itself.

I think adding "(fn ARG1 ARG2...)" to the docstring would solve (1)-(3).
What's lacking is a way to add this automagically like DEFUN does.  And
getting the parameters in C variables like DEFUN would also be nice.





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#23486: 25.0.93; Modules: features missing from make_function
  2016-09-11 14:57 ` npostavs
@ 2017-03-26 20:02   ` Philipp Stephani
  2017-03-26 20:22     ` npostavs
  0 siblings, 1 reply; 15+ messages in thread
From: Philipp Stephani @ 2017-03-26 20:02 UTC (permalink / raw)
  To: npostavs; +Cc: 23486

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

<npostavs@users.sourceforge.net> schrieb am So., 11. Sep. 2016 um 16:56 Uhr:

> Philipp Stephani <p.stephani2@gmail.com> writes:
>
> > emacs_env::make_function lacks the following features supported by
> > `defun':
> >
> > 1. Functions with both optional and rest arguments.
> > 2. Specification of parameter names.
> > 3. Integration with `help-function-arglist'.
> > 4. Specification of interactive forms.
> > 5. Specification of declare forms.
> > 6. Docstrings containing null or non-Unicode characters.
> >
> > (6) is probably rather unimportant.  (5) is probably not implementable
> > (would require wrapping `defun', not `lambda').  (1)–(4) are more severe
> > and quite limit the usefulness of make_function right now; for a
> > truly generic `defun'-like construct one currently has to eval a `defun'
> > form wrapping another function.
>
> Shouldn't modules be providing a DEFUN-like construct instead?  That is,
> I thought the idea of modules was to enable writing primitive
> subroutines.
>

I don't know what the idea of modules originally was. However, defun and
DEFUN are composite operations: They create a function object (lambda) and
provide an alias for it. Therefore they can't replace the more primitive
operations. The current module interface design chooses to provide the
primitive operation to make a function object and have the caller call
defalias. That's a reasonable choice.


>
> >
> > To solve (1)–(3), I'd propose replacing the "arity" arguments with a
> > true arglist specification.  This could either be at the C level, e.g.
> >
> >     ptrdiff_t num_mandatory_args, char** mandatory_arg_names,
> >     ptrdiff_t num_optional_args, char** optional_arg_names,
> >     char* rest_arg_name
> >
> > or by requiring to pass a Lisp argument list.
> >
> > To solve (4) I'd propose to pass another value for the interactive form,
> > probably as emacs_value* (to support non-interactive functions).
> >
> > As an alternative, if people feel this would require too many
> > parameters, I'd propose reverting the change that adds the documentation
> > string.  A docstring without arglist is not very useful.  We could also
> > remove the arity parameters and have the C function check the arity
> > itself.
>
> I think adding "(fn ARG1 ARG2...)" to the docstring would solve (1)-(3).
>

That doesn't work, because Emacs ignores this syntax when the arguments are
provided explicitly, and since a module function is just a (lambda (&rest
args) ...) under the hood, the arglist is always just (&rest args).


> What's lacking is a way to add this automagically like DEFUN does.  And
> getting the parameters in C variables like DEFUN would also be nice.
>

Maybe, but not for the module interface. The module interface explicitly
only provides basic primitives, without macro magic or high-level
functions. High-level functionality built on top of the primitives is out
of scope.

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#23486: 25.0.93; Modules: features missing from make_function
  2017-03-26 20:02   ` Philipp Stephani
@ 2017-03-26 20:22     ` npostavs
  2017-03-26 20:40       ` Philipp Stephani
  0 siblings, 1 reply; 15+ messages in thread
From: npostavs @ 2017-03-26 20:22 UTC (permalink / raw)
  To: Philipp Stephani; +Cc: 23486

Philipp Stephani <p.stephani2@gmail.com> writes:

>
>>  I think adding "(fn ARG1 ARG2...)" to the docstring would solve (1)-(3).
>
> That doesn't work, because Emacs ignores this syntax when the
> arguments are provided explicitly, and since a module function is just
> a (lambda (&rest args) ...) under the hood, the arglist is always just
> (&rest args).

I don't know what you mean here.

    (defun foo (&rest args)
      "Do foo.

    \(fn ARG1 ARG2)")

<f1> f foo RET gives

    foo is a Lisp function.

    (foo ARG1 ARG2)

    Do foo.





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#23486: 25.0.93; Modules: features missing from make_function
  2017-03-26 20:22     ` npostavs
@ 2017-03-26 20:40       ` Philipp Stephani
  0 siblings, 0 replies; 15+ messages in thread
From: Philipp Stephani @ 2017-03-26 20:40 UTC (permalink / raw)
  To: npostavs; +Cc: 23486

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

<npostavs@users.sourceforge.net> schrieb am So., 26. März 2017 um 22:21 Uhr:

> Philipp Stephani <p.stephani2@gmail.com> writes:
>
> >
> >>  I think adding "(fn ARG1 ARG2...)" to the docstring would solve
> (1)-(3).
> >
> > That doesn't work, because Emacs ignores this syntax when the
> > arguments are provided explicitly, and since a module function is just
> > a (lambda (&rest args) ...) under the hood, the arglist is always just
> > (&rest args).
>
> I don't know what you mean here.
>
>     (defun foo (&rest args)
>       "Do foo.
>
>     \(fn ARG1 ARG2)")
>
> <f1> f foo RET gives
>
>     foo is a Lisp function.
>
>     (foo ARG1 ARG2)
>
>     Do foo.
>

OK, that one works, but others don't (e.g. help-function-arglist). The
argument names should be transparent, without having to use such tricks.

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#23486: 25.0.93; Modules: features missing from make_function
  2016-05-09 16:37 bug#23486: 25.0.93; Modules: features missing from make_function Philipp Stephani
  2016-09-11 14:13 ` Philipp Stephani
  2016-09-11 14:57 ` npostavs
@ 2017-03-27  3:57 ` npostavs
  2017-07-04 18:20   ` Philipp Stephani
  2 siblings, 1 reply; 15+ messages in thread
From: npostavs @ 2017-03-27  3:57 UTC (permalink / raw)
  To: Philipp Stephani; +Cc: 23486

Philipp Stephani <p.stephani2@gmail.com> writes:

> As an alternative, if people feel this would require too many
> parameters, I'd propose reverting the change that adds the documentation
> string.  A docstring without arglist is not very useful.  We could also
> remove the arity parameters and have the C function check the arity
> itself.

Looking at this a bit closer, I do think this adds too many parameters,
and in particular, requiring to pass in names for positional parameters
just makes no sense.  The names are never used (except for displaying
documentation).

But removing the docstring is not great.  IMO, the right solution here
is to use a subr-like object instead of a lambda, as suggested in a
FIXME in emacs-module.c:

  /* FIXME: Use a bytecompiled object, or even better a subr.  */

Then the arity could be checked with `subr-arity' or similar.





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#23486: 25.0.93; Modules: features missing from make_function
  2017-03-27  3:57 ` npostavs
@ 2017-07-04 18:20   ` Philipp Stephani
  2017-07-05  3:40     ` npostavs
  0 siblings, 1 reply; 15+ messages in thread
From: Philipp Stephani @ 2017-07-04 18:20 UTC (permalink / raw)
  To: npostavs; +Cc: 23486

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

<npostavs@users.sourceforge.net> schrieb am Mo., 27. März 2017 um 05:56 Uhr:

> Philipp Stephani <p.stephani2@gmail.com> writes:
>
> > As an alternative, if people feel this would require too many
> > parameters, I'd propose reverting the change that adds the documentation
> > string.  A docstring without arglist is not very useful.  We could also
> > remove the arity parameters and have the C function check the arity
> > itself.
>
> Looking at this a bit closer, I do think this adds too many parameters,
> and in particular, requiring to pass in names for positional parameters
> just makes no sense.  The names are never used (except for displaying
> documentation).
>
> But removing the docstring is not great.  IMO, the right solution here
> is to use a subr-like object instead of a lambda, as suggested in a
> FIXME in emacs-module.c:
>
>   /* FIXME: Use a bytecompiled object, or even better a subr.  */
>
> Then the arity could be checked with `subr-arity' or similar.
>

This is now done (commit 31fded0370c3aa6d2c4370cae21cdb7475873483). This
fixes (1) through (3). (4) through (6) are still open. That's probably OK
if the limitations are documented; modules can always do the equivalent of
(eval '(defun ...)) to get complete support.

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#23486: 25.0.93; Modules: features missing from make_function
  2017-07-04 18:20   ` Philipp Stephani
@ 2017-07-05  3:40     ` npostavs
  2020-09-05 13:59       ` Lars Ingebrigtsen
  0 siblings, 1 reply; 15+ messages in thread
From: npostavs @ 2017-07-05  3:40 UTC (permalink / raw)
  To: Philipp Stephani; +Cc: 23486

Philipp Stephani <p.stephani2@gmail.com> writes:

> This is now done (commit
> 31fded0370c3aa6d2c4370cae21cdb7475873483). This fixes (1) through
> (3). (4) through (6) are still open. That's probably OK if the
> limitations are documented; modules can always do the equivalent of
> (eval ' (defun ...)) to get complete support.

Lisp_Subr's have an intspec field (4), but I agree it's not really
essential.  As I think you mentioned in the OP, supporting `declare' (5)
is not doable by definition (because the effects of 'declare' operate on
the symbol, not the function object).

Docstrings containing null or non-Unicode characters (6) just seems
completely pointless to me.  Are there any cases using that capability
in Emacs (or outside Emacs)?  I would probably consider them as bugs.





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#23486: 25.0.93; Modules: features missing from make_function
  2017-07-05  3:40     ` npostavs
@ 2020-09-05 13:59       ` Lars Ingebrigtsen
  2020-09-13  9:44         ` Philipp Stephani
  0 siblings, 1 reply; 15+ messages in thread
From: Lars Ingebrigtsen @ 2020-09-05 13:59 UTC (permalink / raw)
  To: npostavs; +Cc: Philipp Stephani, 23486

npostavs@users.sourceforge.net writes:

> Philipp Stephani <p.stephani2@gmail.com> writes:
>
>> This is now done (commit
>> 31fded0370c3aa6d2c4370cae21cdb7475873483). This fixes (1) through
>> (3). (4) through (6) are still open. That's probably OK if the
>> limitations are documented; modules can always do the equivalent of
>> (eval ' (defun ...)) to get complete support.
>
> Lisp_Subr's have an intspec field (4), but I agree it's not really
> essential.  As I think you mentioned in the OP, supporting `declare' (5)
> is not doable by definition (because the effects of 'declare' operate on
> the symbol, not the function object).
>
> Docstrings containing null or non-Unicode characters (6) just seems
> completely pointless to me.  Are there any cases using that capability
> in Emacs (or outside Emacs)?  I would probably consider them as bugs.

This was the final message in this thread.  Philipp's patch fixed the
main problems, and (5) and (6) doesn't sound like something that
can/should be fixed.  Which leaves (4), which didn't seem essential,
either.

So is there more to be done here, or should this bug report be closed?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#23486: 25.0.93; Modules: features missing from make_function
  2020-09-05 13:59       ` Lars Ingebrigtsen
@ 2020-09-13  9:44         ` Philipp Stephani
  2020-09-13 13:20           ` Lars Ingebrigtsen
  0 siblings, 1 reply; 15+ messages in thread
From: Philipp Stephani @ 2020-09-13  9:44 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: 23486, Noam Postavsky

Am Sa., 5. Sept. 2020 um 15:59 Uhr schrieb Lars Ingebrigtsen <larsi@gnus.org>:
>
> npostavs@users.sourceforge.net writes:
>
> > Philipp Stephani <p.stephani2@gmail.com> writes:
> >
> >> This is now done (commit
> >> 31fded0370c3aa6d2c4370cae21cdb7475873483). This fixes (1) through
> >> (3). (4) through (6) are still open. That's probably OK if the
> >> limitations are documented; modules can always do the equivalent of
> >> (eval ' (defun ...)) to get complete support.
> >
> > Lisp_Subr's have an intspec field (4), but I agree it's not really
> > essential.  As I think you mentioned in the OP, supporting `declare' (5)
> > is not doable by definition (because the effects of 'declare' operate on
> > the symbol, not the function object).
> >
> > Docstrings containing null or non-Unicode characters (6) just seems
> > completely pointless to me.  Are there any cases using that capability
> > in Emacs (or outside Emacs)?  I would probably consider them as bugs.
>
> This was the final message in this thread.  Philipp's patch fixed the
> main problems, and (5) and (6) doesn't sound like something that
> can/should be fixed.  Which leaves (4), which didn't seem essential,
> either.
>
> So is there more to be done here, or should this bug report be closed?
>

I'd still like to fix (4), just for completeness's sake. How about
introducing {get,set}_interactive_spec, just like
{get,set}_function_finalizer?





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#23486: 25.0.93; Modules: features missing from make_function
  2020-09-13  9:44         ` Philipp Stephani
@ 2020-09-13 13:20           ` Lars Ingebrigtsen
  2020-09-13 18:50             ` Philipp Stephani
  0 siblings, 1 reply; 15+ messages in thread
From: Lars Ingebrigtsen @ 2020-09-13 13:20 UTC (permalink / raw)
  To: Philipp Stephani; +Cc: 23486, Noam Postavsky

Philipp Stephani <p.stephani2@gmail.com> writes:

> I'd still like to fix (4), just for completeness's sake. How about
> introducing {get,set}_interactive_spec, just like
> {get,set}_function_finalizer?

Sure, sounds logical to me.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#23486: 25.0.93; Modules: features missing from make_function
  2020-09-13 13:20           ` Lars Ingebrigtsen
@ 2020-09-13 18:50             ` Philipp Stephani
  2020-12-07 16:42               ` Lars Ingebrigtsen
  0 siblings, 1 reply; 15+ messages in thread
From: Philipp Stephani @ 2020-09-13 18:50 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: 23486, Noam Postavsky

Am So., 13. Sept. 2020 um 15:20 Uhr schrieb Lars Ingebrigtsen <larsi@gnus.org>:
>
> Philipp Stephani <p.stephani2@gmail.com> writes:
>
> > I'd still like to fix (4), just for completeness's sake. How about
> > introducing {get,set}_interactive_spec, just like
> > {get,set}_function_finalizer?
>
> Sure, sounds logical to me.
>

OK, I've added something similar to master as commit da0e75e741.





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#23486: 25.0.93; Modules: features missing from make_function
  2020-09-13 18:50             ` Philipp Stephani
@ 2020-12-07 16:42               ` Lars Ingebrigtsen
  2020-12-12 14:31                 ` Philipp Stephani
  0 siblings, 1 reply; 15+ messages in thread
From: Lars Ingebrigtsen @ 2020-12-07 16:42 UTC (permalink / raw)
  To: Philipp Stephani; +Cc: 23486, Noam Postavsky

Philipp Stephani <p.stephani2@gmail.com> writes:

> Am So., 13. Sept. 2020 um 15:20 Uhr schrieb Lars Ingebrigtsen <larsi@gnus.org>:
>>
>> Philipp Stephani <p.stephani2@gmail.com> writes:
>>
>> > I'd still like to fix (4), just for completeness's sake. How about
>> > introducing {get,set}_interactive_spec, just like
>> > {get,set}_function_finalizer?
>>
>> Sure, sounds logical to me.
>>
>
> OK, I've added something similar to master as commit da0e75e741.

Re-skimming this thread, I think that covered everything discussed?  So
I'm closing this bug report.  If there's more to be done here, perhaps
opening a new report makes more sense then reopening.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#23486: 25.0.93; Modules: features missing from make_function
  2020-12-07 16:42               ` Lars Ingebrigtsen
@ 2020-12-12 14:31                 ` Philipp Stephani
  0 siblings, 0 replies; 15+ messages in thread
From: Philipp Stephani @ 2020-12-12 14:31 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: 23486, Noam Postavsky

Am Mo., 7. Dez. 2020 um 17:42 Uhr schrieb Lars Ingebrigtsen <larsi@gnus.org>:
>
> Philipp Stephani <p.stephani2@gmail.com> writes:
>
> > Am So., 13. Sept. 2020 um 15:20 Uhr schrieb Lars Ingebrigtsen <larsi@gnus.org>:
> >>
> >> Philipp Stephani <p.stephani2@gmail.com> writes:
> >>
> >> > I'd still like to fix (4), just for completeness's sake. How about
> >> > introducing {get,set}_interactive_spec, just like
> >> > {get,set}_function_finalizer?
> >>
> >> Sure, sounds logical to me.
> >>
> >
> > OK, I've added something similar to master as commit da0e75e741.
>
> Re-skimming this thread, I think that covered everything discussed?  So
> I'm closing this bug report.  If there's more to be done here, perhaps
> opening a new report makes more sense then reopening.
>

Sounds good.





^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2020-12-12 14:31 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-09 16:37 bug#23486: 25.0.93; Modules: features missing from make_function Philipp Stephani
2016-09-11 14:13 ` Philipp Stephani
2016-09-11 14:57 ` npostavs
2017-03-26 20:02   ` Philipp Stephani
2017-03-26 20:22     ` npostavs
2017-03-26 20:40       ` Philipp Stephani
2017-03-27  3:57 ` npostavs
2017-07-04 18:20   ` Philipp Stephani
2017-07-05  3:40     ` npostavs
2020-09-05 13:59       ` Lars Ingebrigtsen
2020-09-13  9:44         ` Philipp Stephani
2020-09-13 13:20           ` Lars Ingebrigtsen
2020-09-13 18:50             ` Philipp Stephani
2020-12-07 16:42               ` Lars Ingebrigtsen
2020-12-12 14:31                 ` Philipp Stephani

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).