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.bugs Subject: bug#70184: [PATCH] Fix + ert for improper alias notification and setting. Also triple notification bug. (3 of 9) Date: Sat, 06 Apr 2024 10:31:45 +0300 Message-ID: <86plv33q1q.fsf@gnu.org> References: Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="16174"; mail-complaints-to="usenet@ciao.gmane.io" Cc: 70184@debbugs.gnu.org To: Robert Burks , Stefan Monnier Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Sat Apr 06 09:33:18 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 1rt0Y9-0003wn-UR for geb-bug-gnu-emacs@m.gmane-mx.org; Sat, 06 Apr 2024 09:33:18 +0200 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1rt0Xq-0005Ng-Mc; Sat, 06 Apr 2024 03:32:58 -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 1rt0Xp-0005NS-Jr for bug-gnu-emacs@gnu.org; Sat, 06 Apr 2024 03:32:57 -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 1rt0Xp-0000uE-AR for bug-gnu-emacs@gnu.org; Sat, 06 Apr 2024 03:32:57 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1rt0Xv-0005cQ-BD for bug-gnu-emacs@gnu.org; Sat, 06 Apr 2024 03:33:03 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Eli Zaretskii Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sat, 06 Apr 2024 07:33:03 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 70184 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 70184-submit@debbugs.gnu.org id=B70184.171238872221313 (code B ref 70184); Sat, 06 Apr 2024 07:33:03 +0000 Original-Received: (at 70184) by debbugs.gnu.org; 6 Apr 2024 07:32:02 +0000 Original-Received: from localhost ([127.0.0.1]:38082 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1rt0Wv-0005XQ-8J for submit@debbugs.gnu.org; Sat, 06 Apr 2024 03:32:02 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:45164) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1rt0Ws-0005WW-Nn for 70184@debbugs.gnu.org; Sat, 06 Apr 2024 03:31:59 -0400 Original-Received: from fencepost.gnu.org ([2001:470:142:3::e]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1rt0Wh-0007s7-BV; Sat, 06 Apr 2024 03:31:47 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org; s=fencepost-gnu-org; h=References:Subject:In-Reply-To:To:From:Date: mime-version; bh=qTHBFlneZ9mKL8lfDmqgm0W+dDZxvMZhYibs2rbSjmQ=; b=OxgAzTx8S7qo DYti6NMrfQpU98QNlvUx1wuJ4DXccLLUSO48yX4Zc58vzslBF5boq1OB1Ho+O4MRsMLXoGz6zTccH STKYk/aQRfjy3ya5gxlLbm030A106Nfpb4wcQGUd0W79UX9Ahg3ZWN/+a4RLtk58UzHAPTXSbPnnK BLr2ZVDrDzVsFCL0WyHd2PCWnyzRbKorqw9LTUy7/tw3ghJkJVCLA34k+AfcxJ399uP/DYt3J4qhU gmdobizjO7cTBgl14EWbJWhYiX6wGO6IlqIWEybD9S1Ka6+h0zvKS7VX9V4h5VMIkBkhkCb9Nwcoy kgkJxPbYX6IJ7zqLBB6Ugw==; In-Reply-To: (message from Robert Burks on Thu, 4 Apr 2024 04:45:09 -0400) 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:282749 Archived-At: > From: Robert Burks > Date: Thu, 4 Apr 2024 04:45:09 -0400 > > Bug00002a (Improper notification when a new alias is made for an unbound variable) > Bug00002b (Triple notification when using set-default on the an alias of an unbound) > ** Bug recreation is at the end > > I have attached a commit that provides a fix and (close to)full coverage > ert for the portion of code where it resided. I also added some notes to > better clarify the meaning of 'new_alias' and 'base_variable'. > (Please note the BUG# placeholder in seven(7) places will need to be updated.) > > There are notes and a link above the code that clearly spelled out what > should be done. However, the code below did not reflect that; it does > now. There seemed to be confusion with the flag variable's enumeration, > as the set functions use 'SET_INTERNAL_BIND' for let binding not for > variable instantiating. > > I said "close to" above in regards to testing; because, both 'new_alias' > and 'base_variable' can be alias chains and that is not tested for. While > 'Fboundp', 'find_symbol_value', and 'set_internal' follow redirection and > everything currently works, there still should be regression testing to > enforce/reflect defined behaviour. > > Variable watchers are only fully functional with interned symbols. However, > a watcher can work with uninterned symbols being used as variable aliases > so long as the aliases are added after the watcher is added to the base > variable. This is because the trapped write flag is copied at the time > of alias definition and does not require "harmonization" across the obarray. > The test I have included use uninternaed symbols as aliases, I have alternate > versions that use global variables as aliases. Using uninterned symbols > allows for the creation of cleanly written and repeatable tests. > (A bit of foreshadowing) Currently defvaralias allows uninterned symbols as > an alias or base (even though broken cases exist, i.e. watchers added after > an uninterned alias). > > Bug Recreation--------------------------------------------------------- > > emacs -Q--------------------------------------------------------------- > > (defvar results nil) > results > > (defvar test) > test > > (add-variable-watcher 'test (lambda (&rest args) (push args results))) > nil > > (defvaralias 'testalias 'test) > test > > results > ((test nil let nil)) > ;; There was no let binding here!!!!!!!!!! > ;; It shouldn't be calling `set' either in this case, as both are unbound > ;; and we are simply assigning a newly created alias to an unbound variable. > ;; The base variable is being neither set nor let bound!!!! > ;; If defining an old variable obsolete we shouldn't be telling > ;; the new variable's watchers that it is being let bound. > ;; The confusion in the code came from the naming in the enum. > > emacs -Q--------------------------------------------------------------- > (defvar results nil) > results > > (defvar test) > test > > (add-variable-watcher 'test (lambda (&rest args) (push args results))) > nil > > (defvaralias 'testalias 'test) > test > > (set-default 'testalias 100) > 100 > > results > ((test 100 set nil) (test 100 set nil) (test nil let nil)) > ;; By combining with the bug prior we get triple notification when > ;; there should only be one 'set' to 100. > ;; My bug fix prior reduces it to two and this fix corrects this completely. > > emacs -Q--------------------------------------------------------------- > (defvar results nil) > results > > (defvar test) > test > > (defvar testalias 50) > testalias > > (add-variable-watcher 'test (lambda (&rest args) (push args results))) > nil > > (defvaralias 'testalias 'test) > test > > (set-default 'testalias 100) > 100 > > results > ((test 100 set nil) (test 100 set nil) (test 50 let nil)) > ;; This example shows improper 'let' that should read set > ;; and double notification that arises from the previous bug. > ;; This example should have a 'set' to 50 and then a 'set' to 100. Stefan, WDYT about this issue and the patch (note that the patch was updated in bug#70189)?