From unknown Thu Aug 14 17:28:16 2025 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-Mailer: MIME-tools 5.509 (Entity 5.509) Content-Type: text/plain; charset=utf-8 From: bug#62761 <62761@debbugs.gnu.org> To: bug#62761 <62761@debbugs.gnu.org> Subject: Status: ruby-add-log-current-method drops some segments when singleton definition references outer module Reply-To: bug#62761 <62761@debbugs.gnu.org> Date: Fri, 15 Aug 2025 00:28:16 +0000 retitle 62761 ruby-add-log-current-method drops some segments when singleto= n definition references outer module reassign 62761 emacs submitter 62761 Dmitry Gutov severity 62761 normal thanks From debbugs-submit-bounces@debbugs.gnu.org Mon Apr 10 17:02:13 2023 Received: (at submit) by debbugs.gnu.org; 10 Apr 2023 21:02:14 +0000 Received: from localhost ([127.0.0.1]:36045 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1plyeT-0005Ru-HL for submit@debbugs.gnu.org; Mon, 10 Apr 2023 17:02:13 -0400 Received: from lists.gnu.org ([209.51.188.17]:32934) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1plyeQ-0005Rm-VX for submit@debbugs.gnu.org; Mon, 10 Apr 2023 17:02:11 -0400 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 1plyeQ-0002dC-Ic for bug-gnu-emacs@gnu.org; Mon, 10 Apr 2023 17:02:10 -0400 Received: from new1-smtp.messagingengine.com ([66.111.4.221]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1plyeO-0007R8-D9 for bug-gnu-emacs@gnu.org; Mon, 10 Apr 2023 17:02:10 -0400 Received: from compute3.internal (compute3.nyi.internal [10.202.2.43]) by mailnew.nyi.internal (Postfix) with ESMTP id A23D1581F9A for ; Mon, 10 Apr 2023 17:02:05 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute3.internal (MEProxy); Mon, 10 Apr 2023 17:02:05 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gutov.dev; h=cc :content-type:content-type:date:date:from:from:in-reply-to :message-id:mime-version:reply-to:sender:subject:subject:to:to; s=fm1; t=1681160525; x=1681164125; bh=R19WYq46lttHdG06VOOfQ86Ey 7/i0uGHqZKLURrm/XI=; b=DYyeWdy8GBJxDE4hRLQV0MZoMwRBlndVun97A2IGv OHulaohoRKtgJ935CFPOi2MGejPUdF1EBMFs6ZANaPsjNIBiYt1pMLoacuhVhp7k Kh7KlnFTmrM9b6mHKWvPWXHKllx+p9dQHkbhgI5Ksnpd4M0KwePHid4OsGAuedwy QSrJ/b+oppYZ6FT7KoHsTYupai/lg/b6jKAZey9fcbuwMaL3TbFvbfyU6jcjPHvP gRu87KJft9/Ae/tivxl7fA65UYSgBkvM/OGmePacu61V6jJjcBjLplGwMQAmiRMc i9aN4jXAUeCeOSUYBaguPg/VEuNA3S4FYQCZCJd/XGN6g== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-type:content-type:date:date :feedback-id:feedback-id:from:from:in-reply-to:message-id :mime-version:reply-to:sender:subject:subject:to:to:x-me-proxy :x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s=fm2; t= 1681160525; x=1681164125; bh=R19WYq46lttHdG06VOOfQ86Ey7/i0uGHqZK LURrm/XI=; b=ZEPglQDtkgEH1DAzuIto9Hat4jL6Zwj5j22XAadNx+3UvllDD4F ph4KN7dqEF4f5Fh0ld4erqFQgeZF3OyUmY4v/Ng6n1BRI6W3PAxCZ0kdj6yxDQLP 5Fn0f9yjyr8GVTYnhO28yLP7qMMpjCA+s31KLRbWtaMBo7ATF0y0uT3xUCqb26Ue IeraL1XJjFS92ETnmJgq2bFgDHKgn+YbQ0gJVNmlmMFMRBU0eMcOfptjRD2CHDEi yQS+Z9tyloqx0osugsUqEkYRRjZR3AtBvq4vOUOq5BxpLelueCkJD34c1AC388sq c3xORHRWtFHKACr1itPnbBo9C7RhyH7bYDw== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvhedrvdekvddgudehhecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecunecujfgurheptgfkffggfgfvhffusehmtderre dtfeejnecuhfhrohhmpeffmhhithhrhicuifhuthhovhcuoegumhhithhrhiesghhuthho vhdruggvvheqnecuggftrfgrthhtvghrnheptddvgedvfeejtdetueeuteehveejtdffte dtjeeijeduvdegveevuefhleetgeefnecuvehluhhsthgvrhfuihiivgeptdenucfrrghr rghmpehmrghilhhfrhhomhepughmihhtrhihsehguhhtohhvrdguvghv X-ME-Proxy: Feedback-ID: i0e71465a:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA for ; Mon, 10 Apr 2023 17:02:04 -0400 (EDT) Content-Type: multipart/mixed; boundary="------------IKyM0JzpLeG3y6wvZPwNqOS9" Message-ID: <9d81bdc8-355d-7d32-8d3c-361ea0ff585c@gutov.dev> Date: Tue, 11 Apr 2023 00:02:03 +0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.9.0 Content-Language: en-US To: bug-gnu-emacs@gnu.org From: Dmitry Gutov Subject: ruby-add-log-current-method drops some segments when singleton definition references outer module Received-SPF: pass client-ip=66.111.4.221; envelope-from=dmitry@gutov.dev; helo=new1-smtp.messagingengine.com X-Spam_score_int: -19 X-Spam_score: -2.0 X-Spam_bar: -- X-Spam_report: (-2.0 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, FROM_FMBLA_NEWDOM28=0.799, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H3=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_PASS=-0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-Spam-Score: -0.9 (/) X-Debbugs-Envelope-To: submit X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.9 (-) This is a multi-part message in MIME format. --------------IKyM0JzpLeG3y6wvZPwNqOS9 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Example: module M module N module C class D def C.foo _ end end end end end (ruby-add-log-current-method) currently returns "M::C.foo" While it should return "M::N::C.foo". Patch attached. This discovery stems from Mattias Engdegård's report (in private) about an ignored return value from `nreverse`. Is this good for emacs-29? It seems pretty safe (with decent test coverage, including the new test), but probably low urgency. Not a regression either. --------------IKyM0JzpLeG3y6wvZPwNqOS9 Content-Type: text/x-patch; charset=UTF-8; name="ruby-add-log-reference-containing.diff" Content-Disposition: attachment; filename="ruby-add-log-reference-containing.diff" Content-Transfer-Encoding: base64 ZGlmZiAtLWdpdCBhL2xpc3AvcHJvZ21vZGVzL3J1YnktbW9kZS5lbCBiL2xpc3AvcHJvZ21v ZGVzL3J1YnktbW9kZS5lbAppbmRleCBiZWNjYjgxODJhNy4uMTE5OWFmNjQ4MjEgMTAwNjQ0 Ci0tLSBhL2xpc3AvcHJvZ21vZGVzL3J1YnktbW9kZS5lbAorKysgYi9saXNwL3Byb2dtb2Rl cy9ydWJ5LW1vZGUuZWwKQEAgLTE5MTEsNyArMTkxMSw3IEBAIHJ1YnktYWRkLWxvZy1jdXJy ZW50LW1ldGhvZAogICAgICAgICAgICAgICAgICAgICAgICAgKHdoaWxlIG1sCiAgICAgICAg ICAgICAgICAgICAgICAgICAgIChpZiAoc3RyaW5nLWVxdWFsIChjYXIgbWwpIChjYXIgbW4p KQogICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgKHNldHEgbWxpc3QgKG5yZXZlcnNl IChjZHIgbWwpKSBtbCBuaWwpKQotICAgICAgICAgICAgICAgICAgICAgICAgICAob3IgKHNl dHEgbWwgKGNkciBtbCkpIChucmV2ZXJzZSBtbGlzdCkpKSkKKyAgICAgICAgICAgICAgICAg ICAgICAgICAgKHNldHEgbWwgKGNkciBtbCkpKSkKICAgICAgICAgICAgICAgICAgICAgICAo aWYgbWxpc3QKICAgICAgICAgICAgICAgICAgICAgICAgICAgKHNldGNkciAobGFzdCBtbGlz dCkgKGJ1dGxhc3QgbW4pKQogICAgICAgICAgICAgICAgICAgICAgICAgKHNldHEgbWxpc3Qg KGJ1dGxhc3QgbW4pKSkpCmRpZmYgLS1naXQgYS90ZXN0L2xpc3AvcHJvZ21vZGVzL3J1Ynkt bW9kZS10ZXN0cy5lbCBiL3Rlc3QvbGlzcC9wcm9nbW9kZXMvcnVieS1tb2RlLXRlc3RzLmVs CmluZGV4IDhhNzVjODNkMmMzLi4xMTczODVlYTNlOCAxMDA2NDQKLS0tIGEvdGVzdC9saXNw L3Byb2dtb2Rlcy9ydWJ5LW1vZGUtdGVzdHMuZWwKKysrIGIvdGVzdC9saXNwL3Byb2dtb2Rl cy9ydWJ5LW1vZGUtdGVzdHMuZWwKQEAgLTU2Nyw2ICs1NjcsMjIgQEAgcnVieS1hZGQtbG9n LWN1cnJlbnQtbWV0aG9kLW5hbWVzcGFjZS1zaG9ydGhhbmQKICAgICAoc2VhcmNoLWJhY2t3 YXJkICJfIikKICAgICAoc2hvdWxkIChzdHJpbmc9IChydWJ5LWFkZC1sb2ctY3VycmVudC1t ZXRob2QpICJDOjpEI2ZvbyIpKSkpCiAKKyhlcnQtZGVmdGVzdCBydWJ5LWFkZC1sb2ctY3Vy cmVudC1tZXRob2Qtc2luZ2xldG9uLXJlZmVyZW5jaW5nLW91dGVyICgpCisgIChydWJ5LXdp dGgtdGVtcC1idWZmZXIgKHJ1YnktdGVzdC1zdHJpbmcKKyAgICAgICAgICAgICAgICAgICAg ICAgICAgIm1vZHVsZSBNCisgICAgICAgICAgICAgICAgICAgICAgICAgIHwgIG1vZHVsZSBO CisgICAgICAgICAgICAgICAgICAgICAgICAgIHwgICAgbW9kdWxlIEMKKyAgICAgICAgICAg ICAgICAgICAgICAgICAgfCAgICAgIGNsYXNzIEQKKyAgICAgICAgICAgICAgICAgICAgICAg ICAgfCAgICAgICAgZGVmIEMuZm9vCisgICAgICAgICAgICAgICAgICAgICAgICAgIHwgICAg ICAgICAgXworICAgICAgICAgICAgICAgICAgICAgICAgICB8ICAgICAgICBlbmQKKyAgICAg ICAgICAgICAgICAgICAgICAgICAgfCAgICAgIGVuZAorICAgICAgICAgICAgICAgICAgICAg ICAgICB8ICAgIGVuZAorICAgICAgICAgICAgICAgICAgICAgICAgICB8ICBlbmQKKyAgICAg ICAgICAgICAgICAgICAgICAgICAgfGVuZCIpCisgICAgKHNlYXJjaC1iYWNrd2FyZCAiXyIp CisgICAgKHNob3VsZCAoc3RyaW5nPSAocnVieS1hZGQtbG9nLWN1cnJlbnQtbWV0aG9kKSAi TTo6Tjo6Qy5mb28iKSkpKQorCiAoZXJ0LWRlZnRlc3QgcnVieS1hZGQtbG9nLWN1cnJlbnQt bWV0aG9kLWFmdGVyLWlubmVyLWNsYXNzICgpCiAgIChydWJ5LXdpdGgtdGVtcC1idWZmZXIg KHJ1YnktdGVzdC1zdHJpbmcKICAgICAgICAgICAgICAgICAgICAgICAgICAgIm1vZHVsZSBN Cg== --------------IKyM0JzpLeG3y6wvZPwNqOS9-- From debbugs-submit-bounces@debbugs.gnu.org Tue Apr 11 01:50:45 2023 Received: (at 62761) by debbugs.gnu.org; 11 Apr 2023 05:50:45 +0000 Received: from localhost ([127.0.0.1]:36366 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1pm6tx-0005tz-BO for submit@debbugs.gnu.org; Tue, 11 Apr 2023 01:50:45 -0400 Received: from eggs.gnu.org ([209.51.188.92]:51250) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1pm6tv-0005t7-CS for 62761@debbugs.gnu.org; Tue, 11 Apr 2023 01:50:44 -0400 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 1pm6tp-0001tZ-9w; Tue, 11 Apr 2023 01:50:37 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org; s=fencepost-gnu-org; h=MIME-version:References:Subject:In-Reply-To:To:From: Date; bh=wnFe+2+dTCzCiEFQBBbC3sQocWGbuUulEJC4AtsG9Lk=; b=aU41qFNKwJ02DKhye0wR aitrT57dycex3chWxdlagLur9kzxs7DKiOUaz9zL8SBEtgcTQ/bzsFnA74iiWop3EwnC/t7Wr+ZP/ Y4PLm+8HAl0/uGuI61Mzv6EG1KbqwcQ0tDmIusJOom5rn+P7XzErKoeDHRwHiCUfwVXy+xYtOirHa NQL2vSuYVrinPdOlGDocGOoxRR0IlYfJJy0mSsSGggvWUTuSJyQjkao9bh5LALfKIOKYxmbxRuUTs ruOdppgksm5ptm/9Id1SGlO/UiWGDwyCmKj2NdS61aBPiIWVMofJz3U1UVu6bUjDBifj62ceE53Ji 86ffyhb+ZRtCcQ==; Received: from [87.69.77.57] (helo=home-c4e4a596f7) by fencepost.gnu.org with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1pm6to-0007bi-PY; Tue, 11 Apr 2023 01:50:37 -0400 Date: Tue, 11 Apr 2023 08:51:18 +0300 Message-Id: <83bkjvarvt.fsf@gnu.org> From: Eli Zaretskii To: Dmitry Gutov In-Reply-To: <9d81bdc8-355d-7d32-8d3c-361ea0ff585c@gutov.dev> (message from Dmitry Gutov on Tue, 11 Apr 2023 00:02:03 +0300) Subject: Re: bug#62761: ruby-add-log-current-method drops some segments when singleton definition references outer module References: <9d81bdc8-355d-7d32-8d3c-361ea0ff585c@gutov.dev> MIME-version: 1.0 Content-type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Spam-Score: -2.3 (--) X-Debbugs-Envelope-To: 62761 Cc: 62761@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -3.3 (---) > Date: Tue, 11 Apr 2023 00:02:03 +0300 > From: Dmitry Gutov > > module M > module N > module C > class D > def C.foo > _ > end > end > end > end > end > > (ruby-add-log-current-method) currently returns "M::C.foo" > > While it should return "M::N::C.foo". Patch attached. > > This discovery stems from Mattias Engdegård's report (in private) about > an ignored return value from `nreverse`. > > Is this good for emacs-29? I guess so. But I wonder: this code was there since ruby-mode.el was added to Emacs in 2008, so are you saying that (cdr ml) can never be nil at this place, or if it is, it's okay to leave ml at the nil value? IOW, the return value of nreverse has been ignored, but what about the nreverse call before that: > --- a/lisp/progmodes/ruby-mode.el > +++ b/lisp/progmodes/ruby-mode.el > @@ -1911,7 +1911,7 @@ ruby-add-log-current-method > (while ml > (if (string-equal (car ml) (car mn)) > (setq mlist (nreverse (cdr ml)) ml nil)) > - (or (setq ml (cdr ml)) (nreverse mlist)))) > + (setq ml (cdr ml)))) > (if mlist > (setcdr (last mlist) (butlast mn)) > (setq mlist (butlast mn)))) Isn't the second nreverse there to undo the first? I guess I'm asking for slightly more detailed rationale for the change you are proposing, to include the analysis of the code involved and where that code is mistaken. For example, why not say (setq mlist (nreverse mlist)) or (setq ml (nreverse mlist)) instead of just removing the 2nd nreverse call? Thanks. From debbugs-submit-bounces@debbugs.gnu.org Tue Apr 11 20:18:09 2023 Received: (at 62761) by debbugs.gnu.org; 12 Apr 2023 00:18:09 +0000 Received: from localhost ([127.0.0.1]:38541 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1pmOBc-0006jP-UR for submit@debbugs.gnu.org; Tue, 11 Apr 2023 20:18:09 -0400 Received: from wnew4-smtp.messagingengine.com ([64.147.123.18]:40921) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1pmOBa-0006it-26 for 62761@debbugs.gnu.org; Tue, 11 Apr 2023 20:18:07 -0400 Received: from compute6.internal (compute6.nyi.internal [10.202.2.47]) by mailnew.west.internal (Postfix) with ESMTP id 2CE292B06DCE; Tue, 11 Apr 2023 20:17:59 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute6.internal (MEProxy); Tue, 11 Apr 2023 20:17:59 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gutov.dev; h=cc :cc:content-transfer-encoding:content-type:content-type:date :date:from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:sender:subject:subject:to:to; s=fm1; t= 1681258678; x=1681262278; bh=i2mYzfrAtLieRm3gXq92lZV6niiuZIs311a aO/ndgUI=; b=pYSYHyH0t+QLXqezLPFsDgpzfung1kT0P85HZ1/pNw9IKXHYxTx 0FxgdCSlMS9jE7FRaINwwGxFUllqhT3I2mVTopy7sl510wvMiLRzz9deD3f8SClk aFI0S0UDLGtwnUtBU2r6I2aIa0TdHHozv63Y0Q/amGOqNnYUrf08oh7AkjE+lO/G fmXK5yCFOSl73CUdbtyrJ207SVn/N/psoT8Xid3wgcMBGAAdSwF0BzXTxtJFnNb/ 99P1Ou1bxzwxnriYu1dAFrptL7ySFHbMFDrPEeLvy+N7vtfm1PqXSBo0MabWC8Xb x5vIpRWO1/3cZRwWZys/iWN3Il+UwDdRMoA== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :content-type:content-type:date:date:feedback-id:feedback-id :from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:sender:subject:subject:to:to:x-me-proxy :x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s=fm3; t= 1681258678; x=1681262278; bh=i2mYzfrAtLieRm3gXq92lZV6niiuZIs311a aO/ndgUI=; b=VpG5uCPnFZmzO1ZZxq8GoG0czvyAKgkLopq+AlhUmqO2hbSc31x eTbsfAohFVZxRlEaQjtftb4wid5XVcd9U1t9g7oaoGZQXwQjyzLakv2qgxevmt9j o+xhGWfPYDQUqELTkQaG4BGrmplV4MXoNEJHLLgocgfZzXdCa7bsogw4d+b0RrUh QnVdKQLeyjVYvrvCsErUo+dftVoqeR6w7ivJXFgYkG8Tl0dZTvv1UN35ky0RYtFt Jy971rQo2DARWuHtEwn1BCYr5aI9QN81yFnwqt6/4RstbYPQfh2u3fFCeVskgDDe PSQv+pMsb4dW3nvod1caeZOrnptQGqPoEEQ== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvhedrvdekhedgfeefucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepkfffgggfuffvvehfhfgjtgfgsehtkeertddtfeejnecuhfhrohhmpeffmhhi thhrhicuifhuthhovhcuoegumhhithhrhiesghhuthhovhdruggvvheqnecuggftrfgrth htvghrnhephfffheeljeffgeffueeghfekkedtfffgheejvdegjeettdduheeufffggfef jeehnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrhhomhepug hmihhtrhihsehguhhtohhvrdguvghv X-ME-Proxy: Feedback-ID: i0e71465a:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Tue, 11 Apr 2023 20:17:56 -0400 (EDT) Message-ID: <0e094cf2-70e0-29c8-30b4-a4a21d78eb62@gutov.dev> Date: Wed, 12 Apr 2023 03:17:55 +0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.9.0 Subject: Re: bug#62761: ruby-add-log-current-method drops some segments when singleton definition references outer module Content-Language: en-US To: Eli Zaretskii References: <9d81bdc8-355d-7d32-8d3c-361ea0ff585c@gutov.dev> <83bkjvarvt.fsf@gnu.org> From: Dmitry Gutov In-Reply-To: <83bkjvarvt.fsf@gnu.org> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Spam-Score: -1.1 (-) X-Debbugs-Envelope-To: 62761 Cc: 62761@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -2.1 (--) On 11/04/2023 08:51, Eli Zaretskii wrote: >> Date: Tue, 11 Apr 2023 00:02:03 +0300 >> From: Dmitry Gutov >> >> module M >> module N >> module C >> class D >> def C.foo >> _ >> end >> end >> end >> end >> end >> >> (ruby-add-log-current-method) currently returns "M::C.foo" >> >> While it should return "M::N::C.foo". Patch attached. >> >> This discovery stems from Mattias Engdegård's report (in private) about >> an ignored return value from `nreverse`. >> >> Is this good for emacs-29? > > I guess so. But I wonder: this code was there since ruby-mode.el was > added to Emacs in 2008, Right. But the condition (if (string-equal (car ml) (car mn)) succeeds in a very rare case to begin with: when the definition uses a baroque, redundant kind of syntax (repeating the name of the constant instead of just using "def self.xxx" inside the appropriate module). And 'nreverse' is only harmful when there are >1 element in the list, meaning the bug also needs there to be at least 2 levels of nesting above said constant (both M and N, in the above example). And ruby-add-log-current-method isn't used often these days anyway, I guess (except from a little package of mine called robe). All this is to say, it's not surprising that this bug was never reported in the recent years. > so are you saying that (cdr ml) can never be > nil at this place, or if it is, it's okay to leave ml at the nil > value? It definitely can be nil (that's when the loop terminates -- as soon as ml reaches that value). > IOW, the return value of nreverse has been ignored, but what about the > nreverse call before that: > >> --- a/lisp/progmodes/ruby-mode.el >> +++ b/lisp/progmodes/ruby-mode.el >> @@ -1911,7 +1911,7 @@ ruby-add-log-current-method >> (while ml >> (if (string-equal (car ml) (car mn)) >> (setq mlist (nreverse (cdr ml)) ml nil)) >> - (or (setq ml (cdr ml)) (nreverse mlist)))) >> + (setq ml (cdr ml)))) >> (if mlist >> (setcdr (last mlist) (butlast mn)) >> (setq mlist (butlast mn)))) > > Isn't the second nreverse there to undo the first? > > I guess I'm asking for slightly more detailed rationale for the change > you are proposing, to include the analysis of the code involved and > where that code is mistaken. For example, why not say > > (setq mlist (nreverse mlist)) > or > (setq ml (nreverse mlist)) > > instead of just removing the 2nd nreverse call? There are, in total, 3 'nreverse' calls inside the surrounding 'let'. In the normal case (when the constant name is "resolved", that is, the search has a hit), the second 'nreverse', visible in the patch context above, is executed. The first one is not visible, and it's unconditional. Your point about "counteracting" is not without merit: one case I didn't test is when the scope matching the name is not found (meaning the constant on which the method is defined fails to "resolve"). Then the middle 'nreverse' is never called, and the list structure stays reversed. But that's basically undecidable code, and even one could write something like this using runtime constant redefinition, our parser cannot decide the full name of the constant the method is being defined at. Here's a slight modification of the patch which leaves just one (optional) 'nreverse' call, which acts on a fully temporary structure, and thus is unable to alter the values held inside mlist unless really intended. It keeps the behavior the same in the previous cases and slightly improves the one above (the "undecidable" one): diff --git a/lisp/progmodes/ruby-mode.el b/lisp/progmodes/ruby-mode.el index beccb8182a7..b252826680c 100644 --- a/lisp/progmodes/ruby-mode.el +++ b/lisp/progmodes/ruby-mode.el @@ -1905,13 +1905,13 @@ ruby-add-log-current-method (progn (unless (string-equal "self" (car mn)) ; def self.foo ;; def C.foo - (let ((ml (nreverse mlist))) + (let ((ml (reverse mlist))) ;; If the method name references one of the ;; containing modules, drop the more nested ones. (while ml (if (string-equal (car ml) (car mn)) (setq mlist (nreverse (cdr ml)) ml nil)) - (or (setq ml (cdr ml)) (nreverse mlist)))) + (setq ml (cdr ml)))) (if mlist (setcdr (last mlist) (butlast mn)) (setq mlist (butlast mn)))) From debbugs-submit-bounces@debbugs.gnu.org Wed Apr 12 02:16:50 2023 Received: (at 62761) by debbugs.gnu.org; 12 Apr 2023 06:16:50 +0000 Received: from localhost ([127.0.0.1]:38784 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1pmTmj-0008E5-NW for submit@debbugs.gnu.org; Wed, 12 Apr 2023 02:16:49 -0400 Received: from eggs.gnu.org ([209.51.188.92]:41144) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1pmTmf-0008Dr-PT for 62761@debbugs.gnu.org; Wed, 12 Apr 2023 02:16:47 -0400 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 1pmTma-0006UN-79; Wed, 12 Apr 2023 02:16:40 -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=bWuMDOzDlK3hfEUbvvs1x5QLpZDnIzvg0Z9Z3G1FOkw=; b=gLz7TFmIS+hF 9S0EWpLR4whqp/SsUX4vuGWS89rXy7yFcDOC1+burKzn3xYLEUGHUyS+kFTF3SJkevquox4fc+Z/9 LqNkE3oqUdtsr1zVJT/arjPIC1vCeB7WdVUqq7QJsD0Oq4fXSvytETKh5XlWeRRgQvQG/FoUU1NEk 2k83Tk/f2u/crzK+e17VFBXqZw0cyFs9qzeW/2eaZAjabRJV4hikZGkjZZxpHKcq661vjtYopBroL Arso6xnt6RKOoxMpkQEx8TiHfUCD8Pg094SwZIxyOhOqKLu9dm38FXRuvS3NtXetXgJQ7qzMEr2sY 9VTjOQ2Zg3ZlwD8RiAKqOQ==; Received: from [87.69.77.57] (helo=home-c4e4a596f7) by fencepost.gnu.org with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1pmTmZ-00065t-KO; Wed, 12 Apr 2023 02:16:39 -0400 Date: Wed, 12 Apr 2023 09:17:23 +0300 Message-Id: <83ile18w0c.fsf@gnu.org> From: Eli Zaretskii To: Dmitry Gutov In-Reply-To: <0e094cf2-70e0-29c8-30b4-a4a21d78eb62@gutov.dev> (message from Dmitry Gutov on Wed, 12 Apr 2023 03:17:55 +0300) Subject: Re: bug#62761: ruby-add-log-current-method drops some segments when singleton definition references outer module References: <9d81bdc8-355d-7d32-8d3c-361ea0ff585c@gutov.dev> <83bkjvarvt.fsf@gnu.org> <0e094cf2-70e0-29c8-30b4-a4a21d78eb62@gutov.dev> X-Spam-Score: -2.3 (--) X-Debbugs-Envelope-To: 62761 Cc: 62761@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -3.3 (---) > Date: Wed, 12 Apr 2023 03:17:55 +0300 > Cc: 62761@debbugs.gnu.org > From: Dmitry Gutov > > Here's a slight modification of the patch which leaves just one > (optional) 'nreverse' call, which acts on a fully temporary structure, > and thus is unable to alter the values held inside mlist unless really > intended. It keeps the behavior the same in the previous cases and > slightly improves the one above (the "undecidable" one): LGTM, thanks. From debbugs-submit-bounces@debbugs.gnu.org Wed Apr 12 17:47:51 2023 Received: (at 62761-done) by debbugs.gnu.org; 12 Apr 2023 21:47:51 +0000 Received: from localhost ([127.0.0.1]:41989 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1pmiJi-0001wV-Q8 for submit@debbugs.gnu.org; Wed, 12 Apr 2023 17:47:51 -0400 Received: from new3-smtp.messagingengine.com ([66.111.4.229]:33147) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1pmiJf-0001vM-0P for 62761-done@debbugs.gnu.org; Wed, 12 Apr 2023 17:47:49 -0400 Received: from compute2.internal (compute2.nyi.internal [10.202.2.46]) by mailnew.nyi.internal (Postfix) with ESMTP id E865658236D; Wed, 12 Apr 2023 17:47:40 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute2.internal (MEProxy); Wed, 12 Apr 2023 17:47:40 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gutov.dev; h=cc :cc:content-transfer-encoding:content-type:content-type:date :date:from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:sender:subject:subject:to:to; s=fm1; t= 1681336060; x=1681339660; bh=A2WcxNOj2XJGLp1+E1DfQzRU8q6x8ZsEnoD 8xV77KUQ=; b=OC8yWpao+K9ErzZPllx7uJkvLmHNH7OhjOjKc4AfHxfc26NM/my 174g8TWYWZu+dRgvBWGqrRY/qZCvaaoWrKPBz2IYTRh4gFW0zp2+ZkOtdOCdA4oI j9mUWMnONC1VkbFcISAxLLivjdaw7H8NAQA80pkn7/f5kV0HjAZnhUcld7anGozP G/MJQLwgMuwDPJyddbN3borpKS4YSWjkHpeNgXc762aqc33lHbGM2g2PkcEQY8qI NJQhoAWDMrc3zDwDRn/DzUZT6c4qPTiCYH/PJWhCbN5Htfciogx5p5RRyyFEhRBN gs4uNX1ZI9t+Ye5Q0hnGeuZOojDF51sIyXw== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :content-type:content-type:date:date:feedback-id:feedback-id :from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:sender:subject:subject:to:to:x-me-proxy :x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s=fm3; t= 1681336060; x=1681339660; bh=A2WcxNOj2XJGLp1+E1DfQzRU8q6x8ZsEnoD 8xV77KUQ=; b=g+CEB7INSDL8LcaEQu5tOnVFJmRgc9Oky1OWrbltMomMc41ovuO 5wvnKznrEIxOWraRJhwKX1nKZ1TDEyfPSgV5VHclJHOEgbTaXsnfeAsTn/f+rqVH 6vP7BqMGL+194h4m77VW5wreTECBb8GkMJWmOFsOiMsJe1ifePxH+WH14+bKnBcD v7zEA0JdsPoSMDnOEXIm5ifKumX3lfCNcC/5Tty8PqOcrRoI+1U3nOqlee7Q0sKU 1QBW0GtdeXDk46Qx4xbVdxmPah3RS6xM9ajsSxw/PTqLWcH1rjC/1wgikiBW8QG+ MtqY59PgQqXiHc6jbuzOuqMDBQo9jq3km5w== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvhedrvdekjedgtdefucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepkfffgggfuffvvehfhfgjtgfgsehtjeertddtfeejnecuhfhrohhmpeffmhhi thhrhicuifhuthhovhcuoegumhhithhrhiesghhuthhovhdruggvvheqnecuggftrfgrth htvghrnhepiefgteevheevveffheeltdeukeeiieekueefgedugfefgefhudelgfefveel vdevnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrhhomhepug hmihhtrhihsehguhhtohhvrdguvghv X-ME-Proxy: Feedback-ID: i0e71465a:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Wed, 12 Apr 2023 17:47:39 -0400 (EDT) Message-ID: <2ca3dbe9-3a12-362c-df1b-f247ce728b5a@gutov.dev> Date: Thu, 13 Apr 2023 00:47:37 +0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.9.0 Subject: Re: bug#62761: ruby-add-log-current-method drops some segments when singleton definition references outer module Content-Language: en-US To: Eli Zaretskii References: <9d81bdc8-355d-7d32-8d3c-361ea0ff585c@gutov.dev> <83bkjvarvt.fsf@gnu.org> <0e094cf2-70e0-29c8-30b4-a4a21d78eb62@gutov.dev> <83ile18w0c.fsf@gnu.org> From: Dmitry Gutov In-Reply-To: <83ile18w0c.fsf@gnu.org> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Score: -1.1 (-) X-Debbugs-Envelope-To: 62761-done Cc: 62761-done@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -2.1 (--) Version: 29.1 On 12/04/2023 09:17, Eli Zaretskii wrote: >> Date: Wed, 12 Apr 2023 03:17:55 +0300 >> Cc:62761@debbugs.gnu.org >> From: Dmitry Gutov >> >> Here's a slight modification of the patch which leaves just one >> (optional) 'nreverse' call, which acts on a fully temporary structure, >> and thus is unable to alter the values held inside mlist unless really >> intended. It keeps the behavior the same in the previous cases and >> slightly improves the one above (the "undecidable" one): > LGTM, thanks. Installed, marking as done. From unknown Thu Aug 14 17:28:16 2025 Received: (at fakecontrol) by fakecontrolmessage; To: internal_control@debbugs.gnu.org From: Debbugs Internal Request Subject: Internal Control Message-Id: bug archived. Date: Thu, 11 May 2023 11:24:04 +0000 User-Agent: Fakemail v42.6.9 # This is a fake control message. # # The action: # bug archived. thanks # This fakemail brought to you by your local debbugs # administrator