From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Stefan Monnier Newsgroups: gmane.emacs.devel Subject: Re: [PATCH] Add gv-define-expander for plist-get Date: Sat, 05 Sep 2020 14:03:36 -0400 Message-ID: References: Mime-Version: 1.0 Content-Type: text/plain Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="33385"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.0.50 (gnu/linux) Cc: "emacs-devel@gnu.org" To: Naoya Yamashita Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Sat Sep 05 20:04:28 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 1kEcY7-0008Zr-8X for ged-emacs-devel@m.gmane-mx.org; Sat, 05 Sep 2020 20:04:27 +0200 Original-Received: from localhost ([::1]:43718 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1kEcY6-0007tL-AW for ged-emacs-devel@m.gmane-mx.org; Sat, 05 Sep 2020 14:04:26 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:36718) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1kEcXQ-0007Rj-BX for emacs-devel@gnu.org; Sat, 05 Sep 2020 14:03:44 -0400 Original-Received: from mailscanner.iro.umontreal.ca ([132.204.25.50]:45996) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1kEcXO-0003md-1P for emacs-devel@gnu.org; Sat, 05 Sep 2020 14:03:43 -0400 Original-Received: from pmg2.iro.umontreal.ca (localhost.localdomain [127.0.0.1]) by pmg2.iro.umontreal.ca (Proxmox) with ESMTP id 37F0C80C98; Sat, 5 Sep 2020 14:03:40 -0400 (EDT) Original-Received: from mail01.iro.umontreal.ca (unknown [172.31.2.1]) by pmg2.iro.umontreal.ca (Proxmox) with ESMTP id 456758092B; Sat, 5 Sep 2020 14:03:38 -0400 (EDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=iro.umontreal.ca; s=mail; t=1599329018; bh=pMcV6hWqGN9S4T6rrO2ixSNpkXjJsgF+2DjofJxkKwY=; h=From:To:Cc:Subject:References:Date:In-Reply-To:From; b=ZIsZQZOB2H6gcYLOxVMnJWmQw9RZrtgklIofs8Kipde8amQ4IBShHHsg2lBJ+l7Tx 91OFpmEBQvtRH3IFnPZnvaaAliFU6mf8YHv0NsET5/pue/Nn577acAK9HkathG6Kar aLOd38+pZiDw9wcBTS6s5FJkdseVk/+OxjlW64MWwleQ2UB4Et4dksAZwL+WjA4fI0 XRTFEW6os8rx2i2HmGq+ENKanyxjJOkgHyLQCnRck/a5jxOnPBWWrA1KUeWLz99dHa whoaLHXkN38x08l6sntuF8Be0IkXwgMh8cHiCpwSyQ7iJcz4dor+xgtxwFcfyUxhC/ n6HtUIMNS066w== Original-Received: from alfajor (unknown [45.72.232.131]) by mail01.iro.umontreal.ca (Postfix) with ESMTPSA id 1721412053A; Sat, 5 Sep 2020 14:03:38 -0400 (EDT) In-Reply-To: (Naoya Yamashita's message of "Mon, 17 Aug 2020 18:05:54 +0900") Received-SPF: pass client-ip=132.204.25.50; envelope-from=monnier@iro.umontreal.ca; helo=mailscanner.iro.umontreal.ca X-detected-operating-system: by eggs.gnu.org: First seen = 2020/09/05 14:03:40 X-ACL-Warn: Detected OS = Linux 2.2.x-3.x [generic] X-Spam_score_int: -42 X-Spam_score: -4.3 X-Spam_bar: ---- X-Spam_report: (-4.3 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, RCVD_IN_DNSWL_MED=-2.3, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no 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:254545 Archived-At: Hi, Sorry for not answering earlier. > I find `gv` has `gv-define-expander` for `alist-get`, but > there're no definition for `plist-get` Indeed. > +(gv-define-expander plist-get > + (lambda (do plist prop) > + (macroexp-let2 macroexp-copyable-p key prop > + (gv-letplace (getter setter) plist > + (macroexp-let2 nil p `(plist-member ,getter ,key) > + (funcall do > + `(cadr ,p) > + (lambda (val) > + `(if (plist-member ,plist ,key) (setcar (cdr (plist-member ,plist ,key)) ,val) > + ,(funcall setter `(cons ,key (cons ,val ,getter))))))))))) The patch looks good (with special thanks for the tests), except that in the code above, you shouldn't have those (repeated) `(plist-member ,plist ,key)` since they not only waste time but they will also evaluate `plist` multiple times which could change the semantics in case `plist` has side effects. I think these should refer to `p`, right? Furthermore, I think you need to use ',key rather than just ,key in case the key is a cons or a plain symbol, so maybe you want to add a test that uses symbols rather than keywords: ;; Other function (cl-rotatef) usage for plist-get. (should (equal (let ((target '(a "a" b "b" c "c"))) (cl-rotatef (plist-get target 'b) (plist-get target 'c)) target) '(a "a" b "c" c "b"))) Also, I'd add a `cdr` to the computation of `p` so that you don't need to do that `cdr` in both the getter and the setter. -- Stefan