From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.ciao.gmane.io!not-for-mail From: Paul Eggert Newsgroups: gmane.emacs.devel Subject: =?UTF-8?Q?Re=3a_31395511=3a_=22Don=e2=80=99t_attempt_to_modify_cons?= =?UTF-8?Q?tant_strings=22?= Date: Thu, 4 Jun 2020 12:46:23 -0700 Organization: UCLA Computer Science Department Message-ID: References: <871rmvn7ge.fsf@gmail.com> <87lfl36abx.fsf@gmail.com> <1abe5965-b48e-6dee-1516-c5c233f09d01@cs.ucla.edu> <87d06f5m2c.fsf@gmail.com> <87lfl3f5mj.fsf@tcd.ie> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="------------71DBCD029D7691A50418B6FD" Injection-Info: ciao.gmane.io; posting-host="ciao.gmane.io:159.69.161.202"; logging-data="48667"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.8.0 Cc: =?UTF-8?B?Sm/Do28gVMOhdm9yYQ==?= , emacs-devel To: "Basil L. Contovounesios" , Pip Cet Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Thu Jun 04 21:47:16 2020 Return-path: Envelope-to: ged-emacs-devel@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 1jgvpc-000Ccq-1t for ged-emacs-devel@m.gmane-mx.org; Thu, 04 Jun 2020 21:47:16 +0200 Original-Received: from localhost ([::1]:54010 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1jgvpb-0006g9-2c for ged-emacs-devel@m.gmane-mx.org; Thu, 04 Jun 2020 15:47:15 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:46010) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1jgvor-00068x-WD for emacs-devel@gnu.org; Thu, 04 Jun 2020 15:46:30 -0400 Original-Received: from zimbra.cs.ucla.edu ([131.179.128.68]:42480) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1jgvop-0006Y1-CN for emacs-devel@gnu.org; Thu, 04 Jun 2020 15:46:29 -0400 Original-Received: from localhost (localhost [127.0.0.1]) by zimbra.cs.ucla.edu (Postfix) with ESMTP id A57B916007A; Thu, 4 Jun 2020 12:46:25 -0700 (PDT) Original-Received: from zimbra.cs.ucla.edu ([127.0.0.1]) by localhost (zimbra.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10032) with ESMTP id SgLAL4TnXMrs; Thu, 4 Jun 2020 12:46:24 -0700 (PDT) Original-Received: from localhost (localhost [127.0.0.1]) by zimbra.cs.ucla.edu (Postfix) with ESMTP id 6FA1C1600D1; Thu, 4 Jun 2020 12:46:24 -0700 (PDT) X-Virus-Scanned: amavisd-new at zimbra.cs.ucla.edu Original-Received: from zimbra.cs.ucla.edu ([127.0.0.1]) by localhost (zimbra.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10026) with ESMTP id ld_wL73YdLys; Thu, 4 Jun 2020 12:46:24 -0700 (PDT) Original-Received: from [192.168.1.9] (cpe-23-242-74-103.socal.res.rr.com [23.242.74.103]) by zimbra.cs.ucla.edu (Postfix) with ESMTPSA id 2E5AE16007A; Thu, 4 Jun 2020 12:46:24 -0700 (PDT) Autocrypt: addr=eggert@cs.ucla.edu; prefer-encrypt=mutual; keydata= LS0tLS1CRUdJTiBQR1AgUFVCTElDIEtFWSBCTE9DSy0tLS0tCgptUUlOQkV5QWNtUUJFQURB QXlIMnhvVHU3cHBHNUQzYThGTVpFb243NGRDdmM0K3ExWEEySjJ0QnkycHdhVHFmCmhweHhk R0E5Smo1MFVKM1BENGJTVUVnTjh0TFowc2FuNDdsNVhUQUZMaTI0NTZjaVNsNW04c0thSGxH ZHQ5WG0KQUF0bVhxZVpWSVlYL1VGUzk2ZkR6ZjR4aEVtbS95N0xiWUVQUWRVZHh1NDd4QTVL aFRZcDVibHRGM1dZRHoxWQpnZDdneDA3QXV3cDdpdzdlTnZub0RUQWxLQWw4S1lEWnpiRE5D UUdFYnBZM2VmWkl2UGRlSStGV1FONFcra2doCnkrUDZhdTZQcklJaFlyYWV1YTdYRGRiMkxT MWVuM1NzbUUzUWpxZlJxSS9BMnVlOEpNd3N2WGUvV0szOEV6czYKeDc0aVRhcUkzQUZINmls QWhEcXBNbmQvbXNTRVNORnQ3NkRpTzFaS1FNcjlhbVZQa25qZlBtSklTcWRoZ0IxRApsRWR3 MzRzUk9mNlY4bVp3MHhmcVQ2UEtFNDZMY0ZlZnpzMGtiZzRHT1JmOHZqRzJTZjF0azVlVThN Qml5Ti9iClowM2JLTmpOWU1wT0REUVF3dVA4NGtZTGtYMndCeHhNQWhCeHdiRFZadWR6eERa SjFDMlZYdWpDT0pWeHEya2wKakJNOUVUWXVVR3FkNzVBVzJMWHJMdzYrTXVJc0hGQVlBZ1Jy NytLY3dEZ0JBZndoU In-Reply-To: <87lfl3f5mj.fsf@tcd.ie> Content-Language: en-US Received-SPF: pass client-ip=131.179.128.68; envelope-from=eggert@cs.ucla.edu; helo=zimbra.cs.ucla.edu X-detected-operating-system: by eggs.gnu.org: First seen = 2020/06/04 15:46:25 X-ACL-Warn: Detected OS = Linux 3.1-3.10 X-Spam_score_int: -41 X-Spam_score: -4.2 X-Spam_bar: ---- X-Spam_report: (-4.2 / 5.0 requ) BAYES_00=-1.9, RCVD_IN_DNSWL_MED=-2.3, SPF_PASS=-0.001 autolearn=_AUTOLEARN X-Spam_action: no action X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.23 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-mx.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.io gmane.emacs.devel:251859 Archived-At: This is a multi-part message in MIME format. --------------71DBCD029D7691A50418B6FD Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 6/4/20 4:11 AM, Basil L. Contovounesios wrote: > How would make-text-button detect whether its first argument is mutable= ? It could try to mutate the string, and catch the error that is thrown whe= n it's not mutable. No such error is thrown now and Emacs can crash or worse - b= ut I plan to arrange for one to be thrown. > Would it not suffice to clarify in its documentation that it modifies > its argument, in the same way that we warn about passing immutable list= s > to nconc? We could do that, yes. Some code passes string literals to make-text-butt= on, though, and we'd need to change it. The first example I found was ibuf-ex= t.el's ibuffer-old-saved-filters-warning, which calls (make-text-button "here" .= ..). Such code is already "broken" in some sense, so we'll need to fix it anyw= ay somehow. On 6/4/20 12:26 AM, Pip Cet wrote: > I'm not sure the copy-sequence-unless-mutable semantics really > make sense, though, as that might make bugs such as this one even harde= r > to find. True. > I think we should add a new function with clean semantics, and throw an > error in the old function if the string isn't "mutable", whatever that > means in this context. Throwing an error matches Basil's suggestion. What sort of clean semantic= s did you have in mind? > (I guess I can't modify the string contents or > add text properties, but can I modify existing properties? What about > cons cells deep within the properties? If they're recursively immutable= , > what about markers and other objects that change state behind your > back?) The test I was thinking of is pretty simple: you can't modify the string = object itself, but you can modify the objects it points at. We could come up wit= h fancier tests later involving immutable property lists, but one thing at = a time and maybe this one thing is good enough (at least it should avoid the und= efined behavior). > I'm still surprised my patch fixed the problem here (for some buttons, > at least, for others there are a few more places that do the same > thing...) but not for Jo=C3=A3o. There are several instances of the same problem in SLY. I found the ones = in the attached patch, and I expect there are others. So perhaps Jo=C3=A3o was r= unning into one of the other problems. --------------71DBCD029D7691A50418B6FD Content-Type: text/x-patch; charset=UTF-8; name="sly.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="sly.diff" diff --git a/contrib/sly-mrepl.el b/contrib/sly-mrepl.el index f569bc48..265d752b 100644 --- a/contrib/sly-mrepl.el +++ b/contrib/sly-mrepl.el @@ -539,8 +539,7 @@ BEFORE and AFTER as in `sly-mrepl--save-and-copy-for-repl'" 'part-args (list (cadr result) idx) 'part-label (format "REPL Result") 'sly-mrepl--result result - 'sly-button-search-id (sly-button-next-search-id)) - (car result)) + 'sly-button-search-id (sly-button-next-search-id))) (defun sly-mrepl--insert-results (results) (let* ((comint-preoutput-filter-functions nil)) diff --git a/contrib/sly-stickers.el b/contrib/sly-stickers.el index bd4bda06..7fcb4cc5 100644 --- a/contrib/sly-stickers.el +++ b/contrib/sly-stickers.el @@ -353,8 +353,7 @@ render the underlying text unreadable." :type 'sly-stickers--recording-part 'part-args (list sticker-id recording vindex) 'part-label "Recorded value" - props) - label) + props)) (cl-defun sly-stickers--describe-recording-values (recording &key (indent 0) diff --git a/contrib/sly-trace-dialog.el b/contrib/sly-trace-dialog.el index 44759e01..ec36a895 100644 --- a/contrib/sly-trace-dialog.el +++ b/contrib/sly-trace-dialog.el @@ -363,8 +363,7 @@ inspecting details of traced functions. Invoke this dialog with C-c T." 'part-label (format "%s %s" (capitalize (substring (symbol-name type) 1)) - part-id)) - part-text) + part-id))) (define-button-type 'sly-trace-dialog-spec :supertype 'sly-part 'action 'sly-button-show-source @@ -401,8 +400,7 @@ inspecting details of traced functions. Invoke this dialog with C-c T." 'part-args (list id (cdr (sly-trace-dialog--trace-spec trace))) 'part-label (format "Trace entry: %s" id) - props)) - label) + props))) (defun sly-trace-dialog--draw-tree-lines (start offset direction) (save-excursion diff --git a/lib/sly-buttons.el b/lib/sly-buttons.el index 8297ea74..81d63e7c 100644 --- a/lib/sly-buttons.el +++ b/lib/sly-buttons.el @@ -106,8 +106,7 @@ label nil :type 'sly-action 'action action 'mouse-action action - props) - label) + props)) (defface sly-action-face `((t (:inherit warning))) diff --git a/sly.el b/sly.el index 0ff8c0e0..d6ae2e79 100644 --- a/sly.el +++ b/sly.el @@ -3877,8 +3877,7 @@ SEARCH-FN is either the symbol `search-forward' or `search-backward'." (defun sly--compilation-note-group-button (label notes) "Pepare notes as a `sly-compilation-note' button. For insertion in the `compilation-mode' buffer" - (sly--make-text-button label nil :type 'sly-compilation-note-group 'sly-notes-group notes) - label) + (sly--make-text-button label nil :type 'sly-compilation-note-group 'sly-notes-group notes)) ;;;; Basic arglisting @@ -4568,12 +4567,12 @@ TODO" (car designator))) (defun sly-apropos-insert-symbol (designator item bounds package-designator-searched-p) - (let ((label (sly-apropos-designator-string designator))) - (sly--make-text-button label nil + (let ((label + (sly--make-text-button (sly-apropos-designator-string designator) nil 'face 'sly-apropos-symbol 'part-args (list item nil) 'part-label "Symbol" - :type 'sly-apropos-symbol) + :type 'sly-apropos-symbol))) (cl-loop with offset = (if package-designator-searched-p 0 @@ -4728,8 +4727,7 @@ The most important commands: (sly--make-text-button label nil :type 'sly-xref 'part-args (list location) - 'part-label "Location") - label) + 'part-label "Location")) (defun sly-insert-xrefs (xref-alist) "Insert XREF-ALIST in the current-buffer. @@ -5742,8 +5740,7 @@ If MORE is non-nil, more frames are on the Lisp stack." 'part-args (list (car frame) (sly-db--guess-frame-function frame)) 'part-label (format "Frame %d" (car frame)) - props) - label) + props)) (defun sly-db-frame-number-at-point () (let ((button (sly-db-frame-button-near-point))) @@ -5851,8 +5848,7 @@ If MORE is non-nil, more frames are on the Lisp stack." (apply #'sly--make-text-button label nil :type 'sly-db-local-variable 'part-args (list frame-number var-id) - 'part-label (format "Local Variable %d" var-id) props) - label) + 'part-label (format "Local Variable %d" var-id) props)) (defun sly-db-frame-details-region (frame-button) "Get (BEG END) for FRAME-BUTTON's details, or nil if hidden" @@ -6584,8 +6580,7 @@ was called originally." :type 'sly-inspector-part 'part-args (list id) 'part-label "Inspector Object" - props) - label) + props)) (defmacro sly-inspector-fontify (face string) `(sly-add-face ',(intern (format "sly-inspector-%s-face" face)) ,string)) --------------71DBCD029D7691A50418B6FD--