From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.devel Subject: Re: [PATCH v2 07/16] Take offset not idx in PER_BUFFER_VALUE_P Date: Tue, 01 Dec 2020 19:20:34 +0200 Message-ID: <834kl5igt9.fsf@gnu.org> References: <20201119153814.17541-1-sbaugh@catern.com> Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="25205"; mail-complaints-to="usenet@ciao.gmane.io" Cc: sbaugh@catern.com, arnold@tdrhq.com, dgutov@yandex.ru, monnier@iro.umontreal.ca, emacs-devel@gnu.org To: Spencer Baugh Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Tue Dec 01 18:38:14 2020 Return-path: Envelope-to: ged-emacs-devel@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1kk9bR-0006Tg-EL for ged-emacs-devel@m.gmane-mx.org; Tue, 01 Dec 2020 18:38:13 +0100 Original-Received: from localhost ([::1]:59814 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1kk9bQ-00083H-Dx for ged-emacs-devel@m.gmane-mx.org; Tue, 01 Dec 2020 12:38:12 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:36370) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1kk9KU-0006yI-VH for emacs-devel@gnu.org; Tue, 01 Dec 2020 12:20:45 -0500 Original-Received: from fencepost.gnu.org ([2001:470:142:3::e]:59174) by eggs.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1kk9KU-0002Ob-Eu; Tue, 01 Dec 2020 12:20:42 -0500 Original-Received: from [176.228.60.248] (port=3784 helo=home-c4e4a596f7) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1kk9KS-0004jr-Lo; Tue, 01 Dec 2020 12:20:42 -0500 In-Reply-To: (message from Spencer Baugh on Sat, 21 Nov 2020 21:34:36 -0500) X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.io gmane.emacs.devel:260143 Archived-At: > From: Spencer Baugh > Date: Sat, 21 Nov 2020 21:34:36 -0500 > Cc: Spencer Baugh , Arnold Noronha , > Stefan Monnier , Dmitry Gutov > > This also moves the common "idx == -1" check into PER_BUFFER_VALUE_P. > If idx == -1, it's guaranteed that the corresponding buffer field has > a per-buffer value. We do this check everywhere we call > PER_BUFFER_VALUE_P, so let's put it in the common code. But the comment says: If a slot in this structure is -1, then even though there may be a DEFVAR_PER_BUFFER for the slot, there is no default value for it; and the corresponding slot in buffer_defaults is not used. Doesn't this contradict your assumption, even though the existing uses of the macro don't? > This is motivated by later commits which will get rid of idx. Why do something with idx if we will get rid of it? I don't see any purpose in such incremental changes: we will install the changes regarding buffer-local values as a single changeset, so keeping the code working after each separate patch in the series is not needed, and just makes the changes unnecessarily larger. IOW, let's consider the change which gets rid of idx without this intermediate step. > +/* Value is true if the variable with offset OFFSET has a local value > + in buffer B. */ > + > +INLINE bool > +PER_BUFFER_VALUE_P (struct buffer *b, int offset) > +{ > + int idx = PER_BUFFER_IDX (offset); > + eassert (idx == -1 || valid_per_buffer_idx (idx)); > + return idx == -1 || b->local_flags[idx]; > +} > + [...] > @@ -1440,8 +1440,8 @@ set_internal (Lisp_Object symbol, Lisp_Object newval, Lisp_Object where, > { > int offset = XBUFFER_OBJFWD (innercontents)->offset; > int idx = PER_BUFFER_IDX (offset); > - if (idx > 0 && bindflag == SET_INTERNAL_SET > - && !PER_BUFFER_VALUE_P (buf, idx)) > + if (bindflag == SET_INTERNAL_SET > + && !PER_BUFFER_VALUE_P (buf, offset)) The original code tests idx for a positive value, but your modified PER_BUFFER_VALUE_P macro will only check that it's non-negative. That's not the same thing.