On Пн, Apr 1, 2019 at 15:27, Robert Pluim <rpluim@gmail.com> wrote:
On Mon, 01 Apr 2019 07:37:23 +0300, Eli Zaretskii
<eliz@gnu.org> said:
>> From: Konstantin Kharlamov <Hi-Angel@yandex.ru> Date: Mon, 1
>> Apr 2019 01:37:38 +0300
>>
>> These are mostly fixes of some of LGTM warnings
>>
https://lgtm.com/projects/g/emacs-mirror/emacs/alerts/?mode=tree
>>
>> Except the second patch, where I initially wanted to fix one
>> warning, and as part of it I had to constify a variable to see
>> that it is indeed immutable. And then I figured I could search
>> through the code and find more similar places, where variables
>> weren't marked as const. I like this cleanup because it is
>> simple and trivially testable (i.e. if it compiles, then it's
>> fine). FTR: there's still lots of opportunities for
>> constification, I just stopped at some point.
Eli> Thanks.
Eli> I think the general policy is not to fix those except when
Eli> making other changes in the same function, but I will let
Eli> others comment.
Iʼd prefer it if the effort went to determining if eg the alert for
'type = 2' below was correct or not, proving the constness of
variables is what we have a compiler for.
xterm.c:5346
if (XSCROLL_BAR (bar)->x_window == window_id
&& FRAME_X_DISPLAY (XFRAME (frame)) == display
&& (type = 2
|| (type == 1 && XSCROLL_BAR (bar)->horizontal)
|| (type == 0 && !XSCROLL_BAR (bar)->horizontal)))
return XSCROLL_BAR (bar);
Robert
Well, not everything at once! :) I afraid that if I fix lots of
warnings in one patch-set, it may get stuck in review because of the
amount of changes; besides it's easier for my sanity to send small
patchsets because mailing-list based projects in general tend not to
accept patches too quickly.
Also note: the constness here is not for compiler but for developers.