unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
From: Robert Cochran <robert+Emacs@cochranmail.com>
To: Eli Zaretskii <eliz@gnu.org>
Cc: emacs-devel@gnu.org, Tom Tromey <tom@tromey.com>,
	robert-emacs@cochranmail.com, dmantipov@yandex.ru,
	eggert@cs.ucla.edu
Subject: Re: [PATCH] src/process.c: remove unnecessary setters
Date: Sat, 06 Jan 2018 01:37:41 -0800	[thread overview]
Message-ID: <871sj3iaze.fsf@cochranmail.com> (raw)
In-Reply-To: <83o9m89nce.fsf@gnu.org> (Eli Zaretskii's message of "Fri, 05 Jan 2018 20:23:13 +0200")

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Tom Tromey <tom@tromey.com>
>> Cc: Paul Eggert <eggert@cs.ucla.edu>,  Eli Zaretskii <eliz@gnu.org>,  dmantipov@yandex.ru,  emacs-devel@gnu.org
>> Date: Fri, 05 Jan 2018 10:58:19 -0700
>> 
>> >>>>> "Robert" == Robert Cochran <robert-emacs@cochranmail.com> writes:
>> 
>> Robert> IMO, there's not much of a point in wrapping something so simple in a
>> Robert> funcall. I understand that a good compiler will optimize that away, but
>> Robert> that doesn't really fix the code any.
>> 
>> Robert> Whatever reason for leaving these has apparently faded into history, if
>> Robert> my past self is to be believed:
>> 
>> IIRC those were introduced to support incremental GC.
>
> Indeed.  In general, we have similar setters for window, frame, and
> buffer objects.  Do we want to get rid of all of those?  And if we do,
> does that mean we abandon all hope for migrating to a more modern GC?

If the setters in question are tiny wrappers whose entire body is a
straight assignment to a struct member (like all of the ones I removed
with this patch), then IMO yes, they should go away.

I left both pset_filter() and pset_sentinel() because they had a default
value that was assigned if the value to be assigned was nil. Those
setters have at least a some value and make sense because there is
logic in them, if only a little bit.

But I see absolutely no value in setters that look like the ones I
removed, where the entire body was just an assignment with no logic or
defaults or anything special.

If these kinds of setters become necessary, for doing the GC bookkeeping
that you mention for instance, then by all means put them back once they
do something other that merely set a structure memeber to exactly what
was passed as a value and nothing more.

My intent is that relatively small changes like this help make the C
parts of Emacs less intimidating. I've noticed a general social
perception that the C parts are intimidating, that people generally
don't want to touch it, and that things are getting to a point where
there are less and less people who understand the C parts.

I feel like the appropriate response then, is to find places like this
where some of the accidental complexity can be made to go away. This
hopefully makes it easier to read the code, thus hopefully making it
easier to understand, thus hopefully inspiring confidence to maintain
and extend the C portions of Emacs.

I do not intend for patches of this nature to obstruct future plans for
shiny features such as the aforementioned advanced GC. And if it turns
out in the future that we /need/ to have these setters, then we can put
them back when the time comes.

Thanks,
-- 
~Robert Cochran

GPG Fingerprint - BD0C 5F8B 381C 64F0 F3CE  E7B9 EC9A 872C 41B2 77C2



  reply	other threads:[~2018-01-06  9:37 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-05-29 23:30 [PATCH] src/process.c: remove unnecessary setters Robert Cochran
2017-05-29 23:47 ` Paul Eggert
2017-05-30  1:40   ` Robert Cochran
2017-05-30  5:19     ` Paul Eggert
2017-05-30  6:05       ` Eli Zaretskii
2018-01-04  0:42         ` Robert Cochran
2018-01-04  0:44           ` Paul Eggert
2018-01-04  4:39             ` Robert Cochran
2018-01-04  8:06               ` Paul Eggert
2018-01-05 17:58               ` Tom Tromey
2018-01-05 18:23                 ` Eli Zaretskii
2018-01-06  9:37                   ` Robert Cochran [this message]
2018-01-06 15:03                     ` Eli Zaretskii

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=871sj3iaze.fsf@cochranmail.com \
    --to=robert+emacs@cochranmail.com \
    --cc=dmantipov@yandex.ru \
    --cc=eggert@cs.ucla.edu \
    --cc=eliz@gnu.org \
    --cc=emacs-devel@gnu.org \
    --cc=robert-emacs@cochranmail.com \
    --cc=tom@tromey.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).