From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Gerd =?UTF-8?Q?M=C3=B6llmann?= Newsgroups: gmane.emacs.bugs Subject: bug#73838: 31.0.50; Problems in note_mouse_highlight if -nw Date: Thu, 17 Oct 2024 09:03:13 +0200 Message-ID: References: <86r08goy76.fsf@gnu.org> <86msj3q4t0.fsf@gnu.org> <86iktrpe46.fsf@gnu.org> <86cyjzp9dj.fsf@gnu.org> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="1268"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Cc: 73838@debbugs.gnu.org To: Eli Zaretskii Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Thu Oct 17 09:05:01 2024 Return-path: Envelope-to: geb-bug-gnu-emacs@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 1t1KZB-0000B3-5I for geb-bug-gnu-emacs@m.gmane-mx.org; Thu, 17 Oct 2024 09:05:01 +0200 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1t1KYv-0002YT-OU; Thu, 17 Oct 2024 03:04:45 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1t1KYs-0002YJ-II for bug-gnu-emacs@gnu.org; Thu, 17 Oct 2024 03:04:42 -0400 Original-Received: from debbugs.gnu.org ([2001:470:142:5::43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1t1KYs-0003vL-9y for bug-gnu-emacs@gnu.org; Thu, 17 Oct 2024 03:04:42 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=debbugs.gnu.org; s=debbugs-gnu-org; h=MIME-Version:Date:References:In-Reply-To:From:To:Subject; bh=qHYCtEdVs2/pdoNDVn+PDCLTV7wWZe1qGd7Z6ouTJV0=; b=j9NboCCfjAMSOV9J4K6FPBH2wIHJWR6LPfkoWVTtHbqRip5FNcr5VB+gLXny24ZrumkmsWZh2lyKkguIMMxdmrhtRc3WWBbbF299YBpJ/7xQAX1o/b8YWJF+FHflZ28c2Pa+9/NmAvTnJY7ANBcYljSkPpf2pE9VFCOU2gid8VF6TQYjsJM8QZwf2yejun6RgVVHWoAoD6K/yJgC9L3bUsmR7VBB7+S607ltHxKALJRHW39RzOH3IY7ofhmZyHfYnNg4xR96spsGe7nU2XqnU1uqKQbtW8yYzHImNpWj5wlS79BMW4qkim1gzF1ELWtY9I1IGRmVnY0Rud02dP30Kg==; Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1t1KZC-0007U2-Ii for bug-gnu-emacs@gnu.org; Thu, 17 Oct 2024 03:05:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Gerd =?UTF-8?Q?M=C3=B6llmann?= Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Thu, 17 Oct 2024 07:05:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 73838 X-GNU-PR-Package: emacs Original-Received: via spool by 73838-submit@debbugs.gnu.org id=B73838.172914868628734 (code B ref 73838); Thu, 17 Oct 2024 07:05:02 +0000 Original-Received: (at 73838) by debbugs.gnu.org; 17 Oct 2024 07:04:46 +0000 Original-Received: from localhost ([127.0.0.1]:33180 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1t1KYv-0007TN-AT for submit@debbugs.gnu.org; Thu, 17 Oct 2024 03:04:45 -0400 Original-Received: from mail-wm1-f48.google.com ([209.85.128.48]:60502) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1t1KYt-0007T8-V1 for 73838@debbugs.gnu.org; Thu, 17 Oct 2024 03:04:44 -0400 Original-Received: by mail-wm1-f48.google.com with SMTP id 5b1f17b1804b1-4315df7b43fso661705e9.0 for <73838@debbugs.gnu.org>; Thu, 17 Oct 2024 00:04:23 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1729148597; x=1729753397; darn=debbugs.gnu.org; h=content-transfer-encoding:mime-version:user-agent:message-id:date :references:in-reply-to:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=qHYCtEdVs2/pdoNDVn+PDCLTV7wWZe1qGd7Z6ouTJV0=; b=IFF0h087Qgv/gKtP2tADLxpy6v1bV+BuaugefJP4VQ0FA5Do1encj+SoF0FZq7F/3u 1anwG7npweckRATwZ5lQOYeqDwJOlEpgigLHm2aQ57F5TdVRQ5YbKMt6ybF8qy27L+/e jPogKqtIvnoP3635+RcRzBFIqaE04AZrWw04ftH9D+mfedXgWNBIXw8uXnMunXP1GO4c 726acJxTnH4xZe4ipveT/X/7XE3sGB8dN9xcqYbA+ulNL0Rp8JgLcxyWZ4+fD2XXYjXu yYBPAjx2o0wuIldAZp28X8UToSzsxfIcuvpjUFjHAfdl+d1x/bdlUSmGOE3nB9IRobtX sVVg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1729148597; x=1729753397; h=content-transfer-encoding:mime-version:user-agent:message-id:date :references:in-reply-to:subject:cc:to:from:x-gm-message-state:from :to:cc:subject:date:message-id:reply-to; bh=qHYCtEdVs2/pdoNDVn+PDCLTV7wWZe1qGd7Z6ouTJV0=; b=iulRQFLYLi+kbw4rowS66tFS6IWmJcjBFqFfv0RM1+sg88g7iqQa7HDiPk8WFtIhdn /p3RbAO7KPNfDub0CXcOLhVPQ4gqiOxan01PfczLdOufK/fRirH1q5RL1Kbo5FrbxcSl V+3YKHVBfwcJ9Rx8a/2vKD0HDOrzSaZ78sc36E+MDVhcyL5L0VkXG5WdcI3P0vkFLL47 u6dZLSaAT257XyMilVETex7GELR5m9KMRlobSOdloD6+fG/x7BNQu3cASbRQJV0e3tF5 +AZynCuzY/9OP5UY31bCs/ujghtFji3vqcHKK7cQNgBr0ZkoupEUASRB7PAgxHRm++Q7 mW5w== X-Gm-Message-State: AOJu0YzTmvPMQbHEGm50aHJWfuzJrqUCaoPQ/3PsZuHYiHX9npvqrqus F7nBETbptTKJBhzRhwqP+r0bHvMFjm3J8Y/DBeV6L5Y1CsGwSygJfd7NvQ== X-Google-Smtp-Source: AGHT+IEHCiNORnH1RzoMJF5d9q7eH+x/1MFM59ZmKWv3ehuhFb51GPQiPySGnM2LDRWxa9EQFpFK+g== X-Received: by 2002:a05:600c:4f46:b0:431:5a0e:fa2e with SMTP id 5b1f17b1804b1-4315a0efb12mr9644365e9.21.1729148596752; Thu, 17 Oct 2024 00:03:16 -0700 (PDT) Original-Received: from pro2 (pd9e36829.dip0.t-ipconnect.de. [217.227.104.41]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-43158c39cafsm16049185e9.16.2024.10.17.00.03.14 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 17 Oct 2024 00:03:16 -0700 (PDT) In-Reply-To: <86cyjzp9dj.fsf@gnu.org> (Eli Zaretskii's message of "Thu, 17 Oct 2024 08:48:56 +0300") X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.bugs:293709 Archived-At: Eli Zaretskii writes: >> From: Gerd M=C3=B6llmann >> Cc: 73838@debbugs.gnu.org >> Date: Thu, 17 Oct 2024 07:07:44 +0200 >>=20 >> Eli Zaretskii writes: >>=20 >> > My suggestion is to extend 'struct tty_display_info' so that >> > FRAME_OUTPUT_DATA on TTY frames will not access unrelated memory, when >> > these macros/inline functions are called. Alternatively, we could have >> > the macros/functions (FRAME_INTERNAL_BORDER etc.) test for TTY frame >> > and DTRT. >>=20 >> FRAME_OUTPUT_DATA is meaningful only for window system frames. Each >> window system's "term" header defines it. For example >>=20 >> xterm.h: >> #define FRAME_X_OUTPUT(f) ((f)->output_data.x) >> #define FRAME_OUTPUT_DATA(f) FRAME_X_OUTPUT (f) >>=20 >> nsterm.h: >> #define FRAME_OUTPUT_DATA(f) ((f)->output_data.ns) >>=20 >> and so on. So using FRAME_OUTPUT_DATA is per se only valid if >> FRAME_WINDOW_P. Which is equivalent to FRAME_NS_P in my case, or >> whatever someone uses. >>=20 >> #ifdef HAVE_X_WINDOWS >> #define FRAME_WINDOW_P(f) FRAME_X_P (f) >> #endif >> #ifdef HAVE_NS >> #define FRAME_WINDOW_P(f) FRAME_NS_P(f) >> #endif >> #ifndef FRAME_WINDOW_P >> #define FRAME_WINDOW_P(f) ((void) (f), false) >> #endif > > My suggestion is to change the last part. output_data is defined like > so: > > union output_data > { > struct tty_output *tty; /* From termchar.h. */ > struct x_output *x; /* From xterm.h. */ > struct w32_output *w32; /* From w32term.h. */ > struct ns_output *ns; /* From nsterm.h. */ > struct pgtk_output *pgtk; /* From pgtkterm.h. */ > struct haiku_output *haiku; /* From haikuterm.h. */ > struct android_output *android; /* From androidterm.h. */ > } > output_data; > > So it exists in TTY frames as well, and we could have bitfields in it > that correspond to the window-system's versions of output_data to > specify internal-border and other decorations. We just need to make > sure these bitfields are zero as long as TTY frames don't support > those features. Then we could avoid the FRAME_WINDOW_P tests which > you propose to add, and still have valid and future-proof code. Ok, I think I understand that. That requires some "base class" design so to speak, which all the xy_output types "inherit" from, right? We don't have that ATM, at least not for NS and X. I agree that would be nice. But it's a lot of work, and probably involves changing code for all platforms. Which I can't. > I thought you were proposing a future-proof change that will avoid the > need to dig into these macros and change them if and when TTY frames > gain more functionality. Well how can I put it. I have actually two problems, let my try to explain :-). The immediate problem I'm facing is with tty child frames and xterm-mouse: I'm opening a buffer selection child frame (consult-buffer) and choose a candidate with a mouse click. The candidates are mouse-highlighted. Result is eventually an endless loop in process_mark_stack in the non-MPS GC. (Not using the mouse works just fine.) Enabling chekcing to the max shhows nothing, so I build with ASAN. And I'm getting stuck early because ASAN shows the invalid access via FRAME_OUTPUT_DATA this bug is about. My second, more general, problem is that the different types of frames are handled so differently in our code, and that it's so difficult to get things to work. I think I've spent at least 90% of the time I spent with child frames with that. The internal-border stuff is an example. I tried to add that for tty frames, for the borders, and it was such a mess (even behaving differently if HVE_WINDOW_SYSTEM or not, i.e. configuring --without-ns or --with-ns) that I git reset --hard in a rage in the end :-). I'd be quite interested to improve that situation, but I'd rather get so far that I can tackle my immediate problem. > If that's not what you are suggesting, I > wonder what is wrong with the current code that we need to make > changes which are basically half-solutions. If the problem is access > to unrelated memory, that is easy to fix by just adding enough slack > to tty_output definition, for example. You mean by making sizeof (struct tty_output) =3D=3D sizeof (ns_output) in my case, and let it go? Bloodcurdlingly horrible! I don't even want to think about it. > Adding tests slows down > redisplay, albeit by a very small amount in each case (but these > slow-downs add up, since we use these macros all over the place). > >> I think changing that would be a major surgery. It's probably easier to >> add checks like I did in the patch to FRAME_OUTPUT_DATA if the frame in >> questioin is indeed a window system frame. It can be decided at run-time >> only anyway. > > It might be easier, but if we are going to make changes, why not do it > the right way to begin with? After all, if what's problematic in the > current code is that valgrind or ASAN complain, we could simply regard > that as false positives, since there are no problems in production > builds. A "false positive" that provents me from using ASAN to find a real problem. > >> The other idea is, IIUC, is to make code using FRAME_OUTPUT_DATA like >> this one >>=20 ... >> I have to admit that I don't like that. I don't understand what is wrong >> to check FRAME_WINDOW_P before using something (using FRAME_OUTPUT_DATA) >> that requires FRAME_WINDOW_P to be valid. > > Let me try explaining once again what I think is wrong with testing > FRAME_WINDOW_P in each of those cases. Imagine that someone develops > a feature whereby TTY frames could have scroll bars or fringes. With > the method you propose, we'd then need to change all the places where > the code accesses scroll bars or fringes, and remove the > FRAME_WINDOW_P and similar tests. If some of these tests are > forgotten and not removed/rewritten, we'd have a subtle bug. By > contrast, the way I propose it, whereby tty_output will have extended > to have the corresponding fields, all we'd need is to set those fields > to some non-zero values, and the rest will "just work". > > IOW, my proposal is to make the tests be based on supported > functionalities rather than on some attribute of a frame which _today_ > is associated with the lack of such functionalities, for the same > reasons we prefer testing functionalities with HAVE_SOMETHING to > testing version numbers or OS-specific symbols. Because the > correlation between frame types and available functionalities can > change in the future, and then adjusting the code to such changes is > usually a tedious and error-prone job. We had ample examples of that > when TTY frames learned to display colors, then again when they > learned to show menus and dialogs. We should learn from those > examples. I'm think I understand that. And I can understand that you are not interested unless a "grander" solution is available, maybe something like the "base class" approach I tried to describe above. So be it. I think you can as well close this bug as wontfix. It's unlikely that I'll work in the direction you would like to see in the forseeable future.