all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: Konstantin Kharlamov <hi-angel@yandex.ru>
To: Eli Zaretskii <eliz@gnu.org>
Cc: Paul Eggert <eggert@cs.ucla.edu>, 35062@debbugs.gnu.org
Subject: bug#35062: [PATCH v3 2/3] constify a bit of xterm.c
Date: Sat, 13 Apr 2019 14:30:46 +0300	[thread overview]
Message-ID: <1555155046.2588.0@yandex.ru> (raw)
In-Reply-To: <8336mml3s7.fsf@gnu.org>



В Сб, апр 13, 2019 at 11:15, Eli Zaretskii <eliz@gnu.org> 
написал:
>>  From: Konstantin Kharlamov <Hi-Angel@yandex.ru>
>>  Date: Sun,  7 Apr 2019 05:13:30 +0300
>> 
>>  * src/xterm.c (x_cr_draw_image, x_update_begin,
>>  x_clear_under_internal_border, x_draw_fringe_bitmap,
>>  x_set_glyph_string_clipping, x_draw_glyph_string_background,
>>  x_draw_composite_glyph_string_foreground, x_send_scroll_bar_event): 
>> make
>>  code easier to follow by making explicit that some variables are
>>  immutable. (Bug#35062)
>>  ---
>>  v3: mention functions changed in commit messages, mention the bug
>>  number, and don't mention that it fixes a warning since intention  
>> of
>>  changes is clear either way.
> 
> I'm really struggling with these changes.  My main problem is that I
> don't see how using 'const' here improves the readability and clarity
> of the code.  IMO, if the variable's name doesn't state its purpose,
> adding 'const' won't help much.  And I think compilers nowadays are
> smart enough to deduce this by themselves.

I've had an example somewhere at the beginning of this thread, so let 
me quote myself

> For illustration you can take a look at patch 3/4 here. There, I
> constify min_rows and min_cols; and afterwards I remove a paragraph
> that compares them to a number that wasn't assigned there. This allows
> to not look through the code before the comparison because it's
> immediately obvious: these variables are never changed.
> 
> This is true for reading the code as well, especially when you're
> debugging a problem: you may often wonder "okay, when was the last 
> time
> that variable changed, could it be invalid here?". Then, if it's
> "const" you immediately know the answer, whereas otherwise you have to
> search through all usages of that variable to see when was the last
> time it changed.

Basically, less mutability means easier to read code. Usually, fully 
immutable code (e.g. in languages such as Haskell) keeps algorithm as 
clean as possible, whereas mutability adds points of mental burden 
(i.e. because you don't know without going through whole function 
whether variable was only assigned once, or was it changed somewhere).

Modern systems language Rust by default is immutable, and mutable 
variables has to be explicitly marked as such (unlike it older 
languages, where everything is mutable, and you have to `const`ify 
things).

-----

Either way, if you folks really in doubt, I'm okay with dropping this 
patch. It took me maybe 10 minutes to assemble it, so it's not like 
there's much work went into it. I simply found a place for improvement 
in code, and did that.

I'd like to take this as an opportunity to ask a question: I see Emacs 
C code is using the old style where variables (mostly) are declared at 
the beginning of a function. Is it just a legacy from C89 days (I hope 
so), or is it a mandatory style?

I'm asking because if I gonna work on the code, I'd for sure like to 
encapsulate variables as much as possible, which means I'd rather 
declare them on the first use (as a bonus, this often may allow to 
constify the variable too).







  reply	other threads:[~2019-04-13 11:30 UTC|newest]

Thread overview: 69+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-31 22:36 bug#35062: Patches: trivial cleanups Konstantin Kharlamov
2019-03-31 22:37 ` bug#35062: [PATCH 0/4] Trivial code cleanups Konstantin Kharlamov
2019-03-31 22:37   ` bug#35062: [PATCH 1/4] Remove redundant comparison Konstantin Kharlamov
2019-03-31 22:37   ` bug#35062: [PATCH 2/4] constify a bit of xterm.c Konstantin Kharlamov
2019-03-31 22:37   ` bug#35062: [PATCH 3/4] min_cols/rows is always 0, don't check for it Konstantin Kharlamov
2019-04-01 22:37     ` Noam Postavsky
2019-04-02  0:09       ` Konstantin Kharlamov
2019-04-02 14:46         ` Eli Zaretskii
2019-04-02 20:54           ` Konstantin Kharlamov
2019-04-03  4:45             ` Eli Zaretskii
2019-04-02  0:23       ` bug#35062: [PATCH v2] min_cols/rows is always 0, remove noop actions Konstantin Kharlamov
2019-04-05  7:05         ` Eli Zaretskii
2019-04-05 22:18           ` Konstantin Kharlamov
2019-04-06  7:09             ` Eli Zaretskii
2019-04-07  2:03               ` Konstantin Kharlamov
2019-04-07  2:45                 ` Eli Zaretskii
2019-04-06  7:25             ` Michael Albinus
2019-03-31 22:37   ` bug#35062: [PATCH 4/4] don't compare unsigned to less-than-zero Konstantin Kharlamov
2019-04-01  4:37   ` bug#35062: [PATCH 0/4] Trivial code cleanups Eli Zaretskii
2019-04-01 13:27     ` Robert Pluim
2019-04-01 13:35       ` Konstantin Kharlamov
2019-04-01 13:41         ` Konstantin Kharlamov
2019-04-01 13:43         ` Robert Pluim
2019-04-01 13:51           ` Konstantin Kharlamov
2019-04-01 14:34       ` Eli Zaretskii
2019-04-01 15:04         ` Robert Pluim
2019-04-01 17:37           ` Eli Zaretskii
2019-04-02 20:49 ` bug#35062: [PATCH] Remove redundant multiplication of ch and cw Konstantin Kharlamov
2019-04-05  7:16   ` Eli Zaretskii
2019-04-07  2:13 ` bug#35062: [PATCH v3 1/3] Remove redundant comparison Konstantin Kharlamov
2019-04-07  2:13   ` bug#35062: [PATCH v3 2/3] constify a bit of xterm.c Konstantin Kharlamov
2019-04-13  8:15     ` Eli Zaretskii
2019-04-13 11:30       ` Konstantin Kharlamov [this message]
2019-04-13 11:36         ` Eli Zaretskii
2019-04-20  0:31       ` Paul Eggert
2019-04-20  1:09         ` Konstantin Kharlamov
2019-04-20  1:17           ` Konstantin Kharlamov
2019-04-20  6:53           ` Eli Zaretskii
2019-04-20 10:31             ` Konstantin Kharlamov
2019-04-20 11:01               ` Eli Zaretskii
2019-04-20 11:23                 ` Konstantin Kharlamov
2019-04-20 11:25                   ` Konstantin Kharlamov
2019-04-20 11:47                     ` Konstantin Kharlamov
2019-04-20 11:58                       ` Konstantin Kharlamov
2019-04-20  6:28         ` Eli Zaretskii
2019-04-07  2:13   ` bug#35062: [PATCH v3 3/3] don't compare unsigned to less-than-zero Konstantin Kharlamov
2019-04-13  8:11     ` Eli Zaretskii
2019-04-13  8:06   ` bug#35062: [PATCH v3 1/3] Remove redundant comparison Eli Zaretskii
2019-04-13 18:19     ` Konstantin Kharlamov
2019-04-13 18:24       ` Eli Zaretskii
2019-04-13 18:28         ` Konstantin Kharlamov
2019-04-13 19:19           ` Eli Zaretskii
2019-04-15  3:38             ` Richard Stallman
2019-04-15  6:49               ` Konstantin Kharlamov
2019-04-15 14:32                 ` Eli Zaretskii
2019-04-15 15:01                   ` Konstantin Kharlamov
2019-04-15 15:21                     ` Eli Zaretskii
2019-04-15 17:03                       ` Konstantin Kharlamov
2019-04-15 17:16                         ` Eli Zaretskii
2019-04-15 17:29                           ` Konstantin Kharlamov
2019-04-15 18:21                             ` Eli Zaretskii
2019-04-15 18:14                 ` Richard Stallman
2019-04-15 18:39                   ` Eli Zaretskii
2019-04-15 14:25               ` Eli Zaretskii
2019-04-16 21:27                 ` Richard Stallman
2019-04-17  2:40                   ` Eli Zaretskii
2019-04-17 20:52                     ` Richard Stallman
2019-06-23 18:07 ` bug#35062: Patches: trivial cleanups Lars Ingebrigtsen
2019-06-23 18:34   ` Constantine Kharlamov

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

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

  git send-email \
    --in-reply-to=1555155046.2588.0@yandex.ru \
    --to=hi-angel@yandex.ru \
    --cc=35062@debbugs.gnu.org \
    --cc=eggert@cs.ucla.edu \
    --cc=eliz@gnu.org \
    /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 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.