From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Dmitry Antipov Newsgroups: gmane.emacs.devel Subject: Re: [RFC] some reworking of struct window Date: Fri, 22 Mar 2013 10:13:42 +0400 Message-ID: <514BF696.2020208@yandex.ru> References: <514AD54F.8050309@yandex.ru> <83620kzk8z.fsf@gnu.org> NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Trace: ger.gmane.org 1363932843 23591 80.91.229.3 (22 Mar 2013 06:14:03 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Fri, 22 Mar 2013 06:14:03 +0000 (UTC) Cc: emacs-devel@gnu.org To: Eli Zaretskii Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Fri Mar 22 07:14:29 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 1UIvFE-0002q4-SX for ged-emacs-devel@m.gmane.org; Fri, 22 Mar 2013 07:14:29 +0100 Original-Received: from localhost ([::1]:54778 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UIvEr-0000vM-2D for ged-emacs-devel@m.gmane.org; Fri, 22 Mar 2013 02:14:05 -0400 Original-Received: from eggs.gnu.org ([208.118.235.92]:45054) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UIvEo-0000vH-Ay for emacs-devel@gnu.org; Fri, 22 Mar 2013 02:14:03 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UIvEm-0003RR-Mr for emacs-devel@gnu.org; Fri, 22 Mar 2013 02:14:02 -0400 Original-Received: from forward1h.mail.yandex.net ([2a02:6b8:0:f05::10]:34922) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UIvEj-0003Qp-AU; Fri, 22 Mar 2013 02:13:57 -0400 Original-Received: from smtp2h.mail.yandex.net (smtp2h.mail.yandex.net [84.201.187.145]) by forward1h.mail.yandex.net (Yandex) with ESMTP id 7A5D99E1998; Fri, 22 Mar 2013 10:13:43 +0400 (MSK) Original-Received: from smtp2h.mail.yandex.net (localhost [127.0.0.1]) by smtp2h.mail.yandex.net (Yandex) with ESMTP id 37B691700868; Fri, 22 Mar 2013 10:13:43 +0400 (MSK) Original-Received: from unknown (unknown [37.139.80.10]) by smtp2h.mail.yandex.net (nwsmtp/Yandex) with ESMTP id DgZueemx-DgZKPjif; Fri, 22 Mar 2013 10:13:42 +0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=yandex.ru; s=mail; t=1363932823; bh=DNaTFIlWLOT3SJx4Vbw+XTmPOoKZzlJQU9rbmj7FPbI=; h=Message-ID:Date:From:User-Agent:MIME-Version:To:CC:Subject: References:In-Reply-To:Content-Type:Content-Transfer-Encoding; b=IeboR3ufJPlcGWFKRaY8wNb0mvljDam/QTA1Ih2YgqmeJnAJg9QW+3E3roNv+e/4Q B/Ja+qjeyDipaOsNtmEPXg5jJXtUkCkQhSXLoTNscTvufLuJRLAgHpMygIpqbWohpv QO6cyoEOZcPHClC6+CZ0SZhBH8zdvvbOFAs2CEnc= User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130307 Thunderbird/17.0.4 In-Reply-To: <83620kzk8z.fsf@gnu.org> X-detected-operating-system: by eggs.gnu.org: Error: Malformed IPv6 address (bad octet value). X-Received-From: 2a02:6b8:0:f05::10 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:158051 Archived-At: 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. 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); >> - eassert (NILP (w->hchild) && NILP (w->vchild)); >> + eassert (w->type == WINDOW_LEAF); > > Why do you need to store the WINDOW_LEAF type explicitly, if you can > test w->object for being a buffer? Since sync_window_with_frame_matrix_rows assumes live window, eassert (BUFFERP (w->object)) may be better; but... > And if you do need this type, why > not use it instead of WINDOWP in the various tests above and below? this is not always the same because w->type == WINDOW_xxx_COMBINATION may be true for the deleted window too. >> @@ -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. >> - /* 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. >> +/* Window types, see the comment above. */ >> + >> +enum window_type >> + { >> + WINDOW_LEAF, >> + WINDOW_HORIZONTAL_COMBINATION, >> + WINDOW_VERTICAL_COMBINATION, >> + }; > > The test BUFFERP (w->object) already gives you the same info as > WINDOW_LEAF type, no? Of course, it's possible to avoid enum window_type at all and use something like: unsigned combination : 1; unsigned horizontal : 1; in struct window. Then we will have w->type == WINDOW_LEAF equal to !w->combination; w->type == WINDOW_VERTICAL_COMBINATION equal to w->combination && !w->horizontal; w->type == WINDOW_HORIZONTAL_COMBINATION equal to w->combination && w->horizontal. I'll try this design too. Dmitry