From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.devel Subject: Re: [RFC] some reworking of struct window Date: Fri, 22 Mar 2013 10:47:55 +0200 Message-ID: <837gkz6csk.fsf@gnu.org> References: <514AD54F.8050309@yandex.ru> <83620kzk8z.fsf@gnu.org> <514BF696.2020208@yandex.ru> Reply-To: Eli Zaretskii NNTP-Posting-Host: plane.gmane.org X-Trace: ger.gmane.org 1363942130 5725 80.91.229.3 (22 Mar 2013 08:48:50 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Fri, 22 Mar 2013 08:48:50 +0000 (UTC) Cc: emacs-devel@gnu.org To: Dmitry Antipov Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Fri Mar 22 09:49:17 2013 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1UIxf2-0004a4-8g for ged-emacs-devel@m.gmane.org; Fri, 22 Mar 2013 09:49:16 +0100 Original-Received: from localhost ([::1]:57502 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UIxee-0001Hf-IE for ged-emacs-devel@m.gmane.org; Fri, 22 Mar 2013 04:48:52 -0400 Original-Received: from eggs.gnu.org ([208.118.235.92]:48853) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UIxeb-0001HY-1L for emacs-devel@gnu.org; Fri, 22 Mar 2013 04:48:50 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UIxeV-0004gx-LX for emacs-devel@gnu.org; Fri, 22 Mar 2013 04:48:48 -0400 Original-Received: from mtaout20.012.net.il ([80.179.55.166]:44239) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UIxeV-0004gR-E3 for emacs-devel@gnu.org; Fri, 22 Mar 2013 04:48:43 -0400 Original-Received: from conversion-daemon.a-mtaout20.012.net.il by a-mtaout20.012.net.il (HyperSendmail v2007.08) id <0MK200D000FQL400@a-mtaout20.012.net.il> for emacs-devel@gnu.org; Fri, 22 Mar 2013 10:47:54 +0200 (IST) Original-Received: from HOME-C4E4A596F7 ([87.69.4.28]) by a-mtaout20.012.net.il (HyperSendmail v2007.08) with ESMTPA id <0MK200DON0FTIZ90@a-mtaout20.012.net.il>; Fri, 22 Mar 2013 10:47:54 +0200 (IST) In-reply-to: <514BF696.2020208@yandex.ru> X-012-Sender: halo1@inter.net.il X-detected-operating-system: by eggs.gnu.org: Solaris 10 X-Received-From: 80.179.55.166 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.14 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-bounces+ged-emacs-devel=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.devel:158056 Archived-At: > Date: Fri, 22 Mar 2013 10:13:42 +0400 > From: Dmitry Antipov > CC: emacs-devel@gnu.org > > On 03/21/2013 10:21 PM, Eli Zaretskii wrote: > > >> @@ -838,16 +838,8 @@ > >> { > >> while (w) > >> { > >> - if (!NILP (w->hchild)) > >> - { > >> - eassert (WINDOWP (w->hchild)); > >> - clear_window_matrices (XWINDOW (w->hchild), desired_p); > >> - } > >> - else if (!NILP (w->vchild)) > >> - { > >> - eassert (WINDOWP (w->vchild)); > >> - clear_window_matrices (XWINDOW (w->vchild), desired_p); > >> - } > >> + if (WINDOWP (w->object)) > >> + clear_window_matrices (XWINDOW (w->object), desired_p); > >> else > >> { > >> if (desired_p) > > > > Here, you effectively lost the assertion that w->object can only be a > > window or a buffer. With the new code, if it's neither a window nor a > > buffer, you will behave as if it were a buffer, without checking. > > No, because clear_window_matrices doesn't check whether w->buffer is not > nil and call clear_glyph_matrix anyway. Sorry, I'm not following your reasoning. > I assume that the most of display-related functions should not be called > for the deleted windows. E.g. this code just silences possible error: > > if (WINDOWP (w->object)) > foo (w->object); > else if (BUFFERP (w->object)) > bar (w->object); > > The following is better since XBUFFER implies eassert if --enable-checking: > > if (WINDOWP (w->object)) > foo (w->object); > else > bar (XBUFFER (w->object)); > > Or: > > if (WINDOWP (w->object)) > foo (w->object); > else if (BUFFERP (w->object)) > bar (w->object); > else > emacs_abort (); > > The latter example leaves the conditional call to emacs_abort even without > --enable-checking. I suspect that this is too paranoid, and would prefer: > > eassert (!NILP (w->object)); > if (WINDOWP (w->object)) > foo (w->object); > else > bar (w->object); But the latter is only protected against nil objects, while the code only handles windows and buffers. Any non-nil Lisp object that is neither a window nor a buffer will slip the assertion. If there's an immediate XBUFFER in the 'else' branch, then indeed an assertion is not needed. But in many cases, this one included, there's no such call to XBUFFER at all, or it is much later in the control flow, which makes it harder to find such bugs. > >> @@ -2069,22 +2060,18 @@ > >> if (!NILP (parent) && NILP (w->combination_limit)) > >> { > >> p = XWINDOW (parent); > >> - if (((!NILP (p->vchild) && !NILP (w->vchild)) > >> - || (!NILP (p->hchild) && !NILP (w->hchild)))) > >> + if (p->type == w->type && p->type > WINDOW_LEAF) > > ^^^^^^^^^^^^^^^^^^^^^ > > I think you are not supposed to compare enumerated types, except for > > equality. How exactly the compiler assigns numerical values to > > enumerated types is implementation-defined, I think. > > No. > > "An enumerator with = defines its enumeration constant as the value of the constant > expression. If the first enumerator has no =, the value of its enumeration constant > is 0. Each subsequent enumerator with no = defines its enumeration constant as the > value of the constant expression obtained by adding 1 to the value of the previous > enumeration constant". > > This is from the latest C99 draft (http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf > 6.7.2.2 "Enumeration specifiers"), but I believe it was so since long time ago. OK, so what happens if the enumeration is reshuffled at some later point in time? And why did you need to use > anyway, when != would do the same, and be free of the problem altogether? > >> - /* P's buffer slot may change from nil to a buffer. */ > >> - adjust_window_count (p, 1); > > > > Why did you remove this call? > > Because I also remove wset_buffer (p, Qnil) few lines below, and per-buffer window counters > should be balanced. Sorry, you lost me here.