From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED.blaine.gmane.org!not-for-mail From: Alan Mackenzie Newsgroups: gmane.emacs.devel Subject: Re: /* FIXME: Call signal_after_change! */ in callproc.c. Well, why not? Date: Sat, 4 Jan 2020 22:47:30 +0000 Message-ID: <20200104224730.GB4009@ACM> References: <20191221172324.GA8692@ACM> <83k16pzgzu.fsf@gnu.org> <20191221214751.GB8692@ACM> <83sglcxl1q.fsf@gnu.org> <20191224094724.GA3992@ACM> <837e2lwws9.fsf@gnu.org> <20191229133436.GA10699@ACM> <83eewhhqri.fsf@gnu.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Injection-Info: blaine.gmane.org; posting-host="blaine.gmane.org:195.159.176.226"; logging-data="261281"; mail-complaints-to="usenet@blaine.gmane.org" User-Agent: Mutt/1.10.1 (2018-07-13) Cc: emacs-devel@gnu.org To: Eli Zaretskii Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Sat Jan 04 23:48:10 2020 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([209.51.188.17]) by blaine.gmane.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.89) (envelope-from ) id 1insDJ-0015H4-R3 for ged-emacs-devel@m.gmane.org; Sat, 04 Jan 2020 23:48:09 +0100 Original-Received: from localhost ([::1]:37394 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1insDI-0004EH-1s for ged-emacs-devel@m.gmane.org; Sat, 04 Jan 2020 17:48:08 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:54494) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1insCm-0003oU-BE for emacs-devel@gnu.org; Sat, 04 Jan 2020 17:47:37 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1insCk-0007Kv-MX for emacs-devel@gnu.org; Sat, 04 Jan 2020 17:47:36 -0500 Original-Received: from colin.muc.de ([193.149.48.1]:43198 helo=mail.muc.de) by eggs.gnu.org with smtp (Exim 4.71) (envelope-from ) id 1insCk-0007Gq-C6 for emacs-devel@gnu.org; Sat, 04 Jan 2020 17:47:34 -0500 Original-Received: (qmail 8087 invoked by uid 3782); 4 Jan 2020 22:47:33 -0000 Original-Received: from acm.muc.de (p2E5D57E0.dip0.t-ipconnect.de [46.93.87.224]) by colin.muc.de (tmda-ofmipd) with ESMTP; Sat, 04 Jan 2020 23:47:31 +0100 Original-Received: (qmail 644 invoked by uid 1000); 4 Jan 2020 22:47:30 -0000 Content-Disposition: inline In-Reply-To: <83eewhhqri.fsf@gnu.org> X-Delivery-Agent: TMDA/1.1.12 (Macallan) X-Primary-Address: acm@muc.de X-detected-operating-system: by eggs.gnu.org: FreeBSD 9.x [fuzzy] X-Received-From: 193.149.48.1 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.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.org gmane.emacs.devel:243948 Archived-At: Hello, Eli. On Fri, Jan 03, 2020 at 10:45:53 +0200, Eli Zaretskii wrote: > > Date: Sun, 29 Dec 2019 13:34:36 +0000 > > Cc: emacs-devel@gnu.org > > From: Alan Mackenzie > > Well, the more we can issue balanced before- and after- calls, the > > better. > I wasn't suggesting to produce unbalanced calls, I was suggesting that > calling the hooks in a balanced way twice for the same position cannot > be a serious problem. It might, for example, cause some hook to invalidate some cache unnecessarily. But even if it caused no specific problem, it would cost a needless runtime penalty. [ .... ] > > We have a real existing bug here, and any fix to it runs the risk of > > further bugs. > Sure, but the more complex the fix, the higher the risk. It's > uneconomical to make fixes that are more complex than strictly > necessary, I'm sure you agree. I think I disagree with you generally on this point - I think it is uneconomical to install a simple workaround when a complete fix is only a little more difficult. This change of mine is NOT complicated. [ .... ] > It falls short of what I'd like to see, because it doesn't cover the > situation where this test: > if (display_on_the_fly > && CODING_REQUIRE_DETECTION (&saved_coding) > && ! CODING_REQUIRE_DETECTION (&process_coding)) > causes us to switch from using the 'else' branch to using the 'else if' > branch in the following snippet: > if (!nread) > ; > else if (NILP (BVAR (current_buffer, enable_multibyte_characters)) > && ! CODING_MAY_REQUIRE_DECODING (&process_coding)) > insert_1_both (buf, nread, nread, 0, 1, 0); > else > { /* We have to decode the input. */ > IOW, the commentary you wrote doesn't tell the reader what > insert_1_both, decode_coding_c_string, and del_range_2 do (or don't > do) with regards to the modification hooks, and without that the > comment is incomplete, and doesn't explain the logic of what the code > does. OK. I have to say here, I really don't believe such an extensive commentary is needed here. The code is there, and anybody generally familiar with our C code would understand it without a great deal of difficulty, even the mechanism which prevents a spurious second call to prepare_to_modify_buffer. Surely? Anyhow I've spent quite a bit of time trying to get the level of this comment right, and my current version of it reads as follows: /* CHANGE FUNCTIONS For each iteration of the enclosing while (1) loop which yields data (i.e. nread > 0), before- and after-change-functions are each invoked exactly once. This is done directly from the current function only, by calling prepare_to_modify_buffer and signal_after_change. It is never done by directing another function such as insert_1_both to call them. The call to prepare_to_modify_buffer follows this comment, and there is one call to signal_after_change in each of the branches of the next `else if'. Exceptionally, the insertion into the buffer is aborted at the call to del_range_2 ~45 lines further down, this function removing the newly inserted data. At this stage prepare_to_modify_buffer has been called, but signal_after_change hasn't. A continue statement restarts the enclosing while(1) loop. A second, unwanted, call to `prepare_to_modify_buffer' is inhibited by the test prepared_pos < PT. The data are inserted again, and this time signal_after_change gets called, balancing the previous call to prepare_to_modify_buffer. */ > Thanks (and apologies for a delay in responding). That's OK, I haven't exactly been prompt myself in posting to this thread. -- Alan Mackenzie (Nuremberg, Germany).