[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] src/process.c: remove unnecessary setters

From: Robert Cochran
Subject: Re: [PATCH] src/process.c: remove unnecessary setters
Date: Sat, 06 Jan 2018 01:37:41 -0800
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/27.0.50 (gnu/linux)

Eli Zaretskii <address@hidden> writes:

>> From: Tom Tromey <address@hidden>
>> Cc: Paul Eggert <address@hidden>,  Eli Zaretskii <address@hidden>,  
>> address@hidden,  address@hidden
>> Date: Fri, 05 Jan 2018 10:58:19 -0700
>> >>>>> "Robert" == Robert Cochran <address@hidden> 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.

~Robert Cochran

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

reply via email to

[Prev in Thread] Current Thread [Next in Thread]