all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* Re: [Emacs-diffs] master 52cc9a5: Reimplement inline functions in ERC with define-inline.
       [not found] ` <20171118134155.1BE4320416@vcs0.savannah.gnu.org>
@ 2017-11-18 14:53   ` Stefan Monnier
  2017-11-18 15:16     ` Eli Zaretskii
  2017-11-26 17:06     ` Vibhav Pant
  0 siblings, 2 replies; 4+ messages in thread
From: Stefan Monnier @ 2017-11-18 14:53 UTC (permalink / raw)
  To: emacs-devel; +Cc: Vibhav Pant

>     Reimplement inline functions in ERC with define-inline.

Yay, someone dared to use define-inline even though I haven't managed to
document it yet!
Thanks.

> -(defsubst erc-dcc-unquote-filename (filename)
> -  (erc-replace-regexp-in-string "\\\\\\\\" "\\"
> -                                (erc-replace-regexp-in-string "\\\\\"" "\"" filename t t) t t))
> +(define-inline erc-dcc-unquote-filename (filename)
> +  (inline-quote
> +   (erc-replace-regexp-in-string "\\\\\\\\" "\\"
> +                                 (erc-replace-regexp-in-string "\\\\\"" "\"" ,filename t t) t t)))

Is it worth the trouble to inline this function at all?
I find it hard to believe that the extra function call overhead would
ever be noticeable compared to the time to do the two regexp replacements.

> -(defsubst erc-nickserv-alist-sender (network &optional entry)
> -  (nth 1 (or entry (assoc network erc-nickserv-alist))))
> +(define-inline erc-nickserv-alist-sender (network &optional entry)
> +  (inline-quote (nth 1 (or ,entry (assoc ,network erc-nickserv-alist)))))

When inlined, `entry` will be evaluated before `network`.
Worse yet: `network` won't be evaluated at all if `entry` is non-nil.
So it won't behave the same when inlined than when non-inlined, which
I'd consider as a bug.

AFAICT, `entry` is never passed anyway, so you could just remove it.

> -(defsubst erc-get-server-user (nick)
> +(define-inline erc-get-server-user (nick)
>    "Find the USER corresponding to NICK in the current server's
>  `erc-server-users' hash table."
> -  (erc-with-server-buffer
> -    (gethash (erc-downcase nick) erc-server-users)))
> +  (inline-quote (erc-with-server-buffer
> +		  (gethash (erc-downcase ,nick) erc-server-users))))

This will evaluate `nick` in another buffer than the caller's
current-buffer, so if the argument refers to buffer-local variables the
result may be different when inlined than it would be when not inlined.


        Stefan



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

* Re: [Emacs-diffs] master 52cc9a5: Reimplement inline functions in ERC with define-inline.
  2017-11-18 14:53   ` [Emacs-diffs] master 52cc9a5: Reimplement inline functions in ERC with define-inline Stefan Monnier
@ 2017-11-18 15:16     ` Eli Zaretskii
  2017-11-26 17:06     ` Vibhav Pant
  1 sibling, 0 replies; 4+ messages in thread
From: Eli Zaretskii @ 2017-11-18 15:16 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: vibhavp, emacs-devel

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Date: Sat, 18 Nov 2017 09:53:44 -0500
> Cc: Vibhav Pant <vibhavp@gmail.com>
> 
> >     Reimplement inline functions in ERC with define-inline.
> 
> Yay, someone dared to use define-inline even though I haven't managed to
> document it yet!

It's documented in the ELisp manual, although I guess what's there can
be improved.



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

* Re: [Emacs-diffs] master 52cc9a5: Reimplement inline functions in ERC with define-inline.
  2017-11-18 14:53   ` [Emacs-diffs] master 52cc9a5: Reimplement inline functions in ERC with define-inline Stefan Monnier
  2017-11-18 15:16     ` Eli Zaretskii
@ 2017-11-26 17:06     ` Vibhav Pant
  2017-11-26 20:23       ` Stefan Monnier
  1 sibling, 1 reply; 4+ messages in thread
From: Vibhav Pant @ 2017-11-26 17:06 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel@gnu.org

On Sat, Nov 18, 2017 at 8:23 PM, Stefan Monnier
<monnier@iro.umontreal.ca> wrote:
> Yay, someone dared to use define-inline even though I haven't managed to
> document it yet!

The existing documentation in inline.el is quite easy to understand. I have also
used it in other packages.

> > -(defsubst erc-nickserv-alist-sender (network &optional entry)
> > -  (nth 1 (or entry (assoc network erc-nickserv-alist))))
> > +(define-inline erc-nickserv-alist-sender (network &optional entry)
> > +  (inline-quote (nth 1 (or ,entry (assoc ,network erc-nickserv-alist)))))
>
> When inlined, `entry` will be evaluated before `network`.
> Worse yet: `network` won't be evaluated at all if `entry` is non-nil.
> So it won't behave the same when inlined than when non-inlined, which
> I'd consider as a bug.

I've pushed a fix by using `inline-letevals` to enforce the correct
evaluation order.

> > -(defsubst erc-get-server-user (nick)
> > +(define-inline erc-get-server-user (nick)
> >    "Find the USER corresponding to NICK in the current server's
> >  `erc-server-users' hash table."
> > -  (erc-with-server-buffer
> > -    (gethash (erc-downcase nick) erc-server-users)))
> > +  (inline-quote (erc-with-server-buffer
> > +               (gethash (erc-downcase ,nick) erc-server-users))))
>
> This will evaluate `nick` in another buffer than the caller's
> current-buffer, so if the argument refers to buffer-local variables the
> result may be different when inlined than it would be when not inlined.

I'm not entirely sure on how to fix this. Would reverting to defsubst/defun be
the only solution?

Thanks,
Vibhav

-- 
Vibhav Pant
vibhavp@gmail.com



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

* Re: [Emacs-diffs] master 52cc9a5: Reimplement inline functions in ERC with define-inline.
  2017-11-26 17:06     ` Vibhav Pant
@ 2017-11-26 20:23       ` Stefan Monnier
  0 siblings, 0 replies; 4+ messages in thread
From: Stefan Monnier @ 2017-11-26 20:23 UTC (permalink / raw)
  To: Vibhav Pant; +Cc: emacs-devel@gnu.org

>> > -(defsubst erc-get-server-user (nick)
>> > +(define-inline erc-get-server-user (nick)
>> >    "Find the USER corresponding to NICK in the current server's
>> >  `erc-server-users' hash table."
>> > -  (erc-with-server-buffer
>> > -    (gethash (erc-downcase nick) erc-server-users)))
>> > +  (inline-quote (erc-with-server-buffer
>> > +               (gethash (erc-downcase ,nick) erc-server-users))))
>> This will evaluate `nick` in another buffer than the caller's
>> current-buffer, so if the argument refers to buffer-local variables the
>> result may be different when inlined than it would be when not inlined.
> I'm not entirely sure on how to fix this. Would reverting to defsubst/defun be
> the only solution?

inline-letevals will do the trick (because you can put it outside of the
`erc-with-server-buffer`).


        Stefan



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

end of thread, other threads:[~2017-11-26 20:23 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20171118134153.7341.67271@vcs0.savannah.gnu.org>
     [not found] ` <20171118134155.1BE4320416@vcs0.savannah.gnu.org>
2017-11-18 14:53   ` [Emacs-diffs] master 52cc9a5: Reimplement inline functions in ERC with define-inline Stefan Monnier
2017-11-18 15:16     ` Eli Zaretskii
2017-11-26 17:06     ` Vibhav Pant
2017-11-26 20:23       ` Stefan Monnier

Code repositories for project(s) associated with this external index

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

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.