From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Ryan Newsgroups: gmane.emacs.bugs Subject: bug#3984: Fix for #3984 Date: Mon, 16 Sep 2013 20:18:23 -0700 Message-ID: <5237C9FF.1000809@thompsonclan.org> References: <20E00C7675E64356BF2F0B2A7E0ABDB1@us.oracle.com> <5232D333.8030206@thompsonclan.org> <523359CD.2070904@thompsonclan.org> <5233670E.4030703@thompsonclan.org> NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-Trace: ger.gmane.org 1379388328 32209 80.91.229.3 (17 Sep 2013 03:25:28 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Tue, 17 Sep 2013 03:25:28 +0000 (UTC) Cc: 3984@debbugs.gnu.org To: Stefan Monnier Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Tue Sep 17 05:25:30 2013 Return-path: Envelope-to: geb-bug-gnu-emacs@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 1VLlur-0006oW-Gl for geb-bug-gnu-emacs@m.gmane.org; Tue, 17 Sep 2013 05:25:29 +0200 Original-Received: from localhost ([::1]:37940 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VLlum-0007eE-G5 for geb-bug-gnu-emacs@m.gmane.org; Mon, 16 Sep 2013 23:25:24 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:34953) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VLlok-0006by-2g for bug-gnu-emacs@gnu.org; Mon, 16 Sep 2013 23:19:17 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VLloc-0002o1-Fk for bug-gnu-emacs@gnu.org; Mon, 16 Sep 2013 23:19:09 -0400 Original-Received: from debbugs.gnu.org ([140.186.70.43]:34141) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VLloc-0002nw-Ae for bug-gnu-emacs@gnu.org; Mon, 16 Sep 2013 23:19:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.80) (envelope-from ) id 1VLlob-0004ze-RS for bug-gnu-emacs@gnu.org; Mon, 16 Sep 2013 23:19:01 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Ryan Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Tue, 17 Sep 2013 03:19:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 3984 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: Original-Received: via spool by 3984-submit@debbugs.gnu.org id=B3984.137938792119162 (code B ref 3984); Tue, 17 Sep 2013 03:19:01 +0000 Original-Received: (at 3984) by debbugs.gnu.org; 17 Sep 2013 03:18:41 +0000 Original-Received: from localhost ([127.0.0.1]:42434 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1VLloF-0004yz-W8 for submit@debbugs.gnu.org; Mon, 16 Sep 2013 23:18:40 -0400 Original-Received: from mail-pb0-f42.google.com ([209.85.160.42]:62700) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1VLloC-0004yj-P7 for 3984@debbugs.gnu.org; Mon, 16 Sep 2013 23:18:37 -0400 Original-Received: by mail-pb0-f42.google.com with SMTP id un15so4940775pbc.1 for <3984@debbugs.gnu.org>; Mon, 16 Sep 2013 20:18:30 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:message-id:date:from:user-agent:mime-version:to :cc:subject:references:in-reply-to:content-type :content-transfer-encoding; bh=LbusCyUyeOf/HRy5pTsG/4io2YZSoZi+gczG1gBEYAM=; b=eZQbhD3QDtz1FA2r2TR6G0+0qP2/mY3UlyhLohXHmZnViu5jXcJsBKtEyEt4J8Ky6Y lZ8gqd5z0k28IKgF8n/zzRgs7uGMi5GSmek2aZ+WhoKVvqOKostVH8yHi6OB5kj7f/o/ mLc93qyrXZmNEz62YTPUSA23AAhwum6Cu4DLlvSOBgqiqckV06hWOEn+o+kCUJ6uYxtS 3UOAcVbRm9c9x/40GkT9cvHLZVE5A/aorVl3L5n47AvYU2T0uFbVZg1PcGLRTIThFQhn URDg5QrMvI5Kkm43ZQEsrzqM/jnItIP/03Gmnuldk13oftcIzl3C+00CAX4DBEvmEANM /hKA== X-Gm-Message-State: ALoCoQndKWXaxgKMDSKmTC2S3NYENt3iyJynrG1jWh19RwosdkGD3x434jh9btpD/o8HumsGJNGS X-Received: by 10.66.140.40 with SMTP id rd8mr11075524pab.119.1379387910629; Mon, 16 Sep 2013 20:18:30 -0700 (PDT) Original-Received: from [192.168.10.2] (user-0c9ha1q.cable.mindspring.com. [24.152.168.58]) by mx.google.com with ESMTPSA id im2sm34744368pbd.31.1969.12.31.16.00.00 (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Mon, 16 Sep 2013 20:18:29 -0700 (PDT) User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:17.0) Gecko/20130801 Thunderbird/17.0.8 In-Reply-To: X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.15 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 140.186.70.43 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.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.bugs:78477 Archived-At: Ok, I think I figured out how to do it for all advice, but feel free to poke holes in my idea before I get to far implementing it. We have "ad-is-advised" which we can use to find which stack frames correspond to advised functions. We have "ad-get-orig-definition" which we can use to find the original definition of an advised function. We can tell if there is any active advice by testing if the advised function's definition is equal to the original definition. If the function has active advice, we can search down the stack for the original definition of that function and skip over all the stack frames in between. Unfortunately, this requires a search through the call stack frames in the wrong order, so the best strategy may be to make a single pass through the call stack, finding all pairs of advised/original function stack frames, and then cache the result in a dynamically bound variable for the duration of the call to "called-interactively-p". This still requires a complete pass through the entire call stack for each call to "called-interactively-p", which defeats the current "lazy-evaluation" strategy of inspecting the frames one by one, although I guess we can stop at the first occurrence of "call-interactively" on the stack. But I doubt that "called-interactively-p" is a performance hotspot anyway, so this may be acceptable. What do you think? -Ryan On 9/13/13 2:02 PM, Stefan Monnier wrote: >>> Looking at the code in trunk, I see that there is a special hook for >>> functions to decide which stack frames to skip over when looking for >>> call-interactively. I still think they should relax the test for >>> equality to "equal indirect-functions" instead of exactly the symbol >>> call-interactively. > The code does check "equal modulo indirect-functions" in some cases, but > indeed not all. I don't think that replacing the equality check against > `call-interactively' with a check modulo indirect-functions would solve > your problem, tho (that only helps when calling though an alias of > call-interactively, but here the relevant stack frame will be a call to > the # which is not > equal-modulo-indirect-functions to call-interactively since > call-interactively has been redefined to a different functions by the > advice). > > You can probably use called-interactively-p-functions to detect the > # and skip the frames between it and the > corresponding call to `call-interactively'. > > But if you find a cute patch against the current code which makes it > work for you in a cleanish way, do send it here, to see if it can > be included. > >> Actually, I just noticed that in trunk, nadvice.el adds a function to >> "called-interactively-p-functions" to skip advice-related stack frames, but >> this works only for advice on the interactive function, not advice defined >> on call-interactively itself. > Indeed. It doesn't even work for all advices (more specifically it > doesn't work for :around advices, which means it doesn't work for > advices defined via `defadvice' since these all turn into one > big :around "new advice"). > >> Furthermore, from my limited testing it appears that the structure of >> the call stack for advised functions has changes significantly in >> trunk, making my code obsolete. > Indeed, the implementation of advices has been completely changed. > >> The whole thing looks like a work in progress right now. > There's no planned change to it, so I consider it "ready modulo > bug-reports". AFAIK it works "at least as well as before" (it works > better than before in the sense that Edebugging a function with calls > to called-interactively-p should now work correctly). > > `called-interactively-p' is a big ugly hack and only works sometimes. > It can break in all kinds of cases (e.g. it currently won't work in > byte-compiled lexical-binding code within a `catch', or > a `condition-case', or the unwind part of an `unwind-protect'). Also, > the functions called (non-interactively, obviously) by your > `call-interactively' advice will probably think that they're called > interactively (hopefully your advice doesn't call many functions, and > hopefully none of them cares whether it's called interactively or not). > > > Stefan