From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: Gemini Lasswell Newsgroups: gmane.emacs.bugs Subject: bug#25351: Patch for bug#25351: 26.0.50; Testcover doesn't work on code that creates simple lists and modifies them Date: Tue, 26 Sep 2017 14:04:13 -0700 Message-ID: <877ewlb29e.fsf@runbox.com> References: NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" X-Trace: blaine.gmane.org 1506459971 2705 195.159.176.226 (26 Sep 2017 21:06:11 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Tue, 26 Sep 2017 21:06:11 +0000 (UTC) User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.0.60 (gnu/linux) To: 25351@debbugs.gnu.org Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Tue Sep 26 23:06:07 2017 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by blaine.gmane.org with esmtp (Exim 4.84_2) (envelope-from ) id 1dwx3O-0000G0-9v for geb-bug-gnu-emacs@m.gmane.org; Tue, 26 Sep 2017 23:06:06 +0200 Original-Received: from localhost ([::1]:51135 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dwx3V-0002c1-NC for geb-bug-gnu-emacs@m.gmane.org; Tue, 26 Sep 2017 17:06:13 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:55600) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dwx3N-0002bv-Nw for bug-gnu-emacs@gnu.org; Tue, 26 Sep 2017 17:06:07 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dwx3K-0005P7-Gn for bug-gnu-emacs@gnu.org; Tue, 26 Sep 2017 17:06:05 -0400 Original-Received: from debbugs.gnu.org ([208.118.235.43]:53255) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1dwx3K-0005Or-36 for bug-gnu-emacs@gnu.org; Tue, 26 Sep 2017 17:06:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1dwx3J-0000px-Om for bug-gnu-emacs@gnu.org; Tue, 26 Sep 2017 17:06:01 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Gemini Lasswell Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Tue, 26 Sep 2017 21:06:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 25351 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: Original-Received: via spool by 25351-submit@debbugs.gnu.org id=B25351.15064599133163 (code B ref 25351); Tue, 26 Sep 2017 21:06:01 +0000 Original-Received: (at 25351) by debbugs.gnu.org; 26 Sep 2017 21:05:13 +0000 Original-Received: from localhost ([127.0.0.1]:33703 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1dwx2X-0000ox-GI for submit@debbugs.gnu.org; Tue, 26 Sep 2017 17:05:13 -0400 Original-Received: from aibo.runbox.com ([91.220.196.211]:56398) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1dwx2U-0000oo-OF for 25351@debbugs.gnu.org; Tue, 26 Sep 2017 17:05:11 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=runbox.com; s=rbselector1; h=Content-Type:MIME-Version:Message-ID:In-Reply-To:Date: References:Subject:Cc:To:From; bh=UompqiCyt8gnEhZ9Yfy/KvH25LJiQVwfhfWhQvo4N4Q=; b=Cp8pXWWtMw0A4S84EJc4Ugutwf WN7u8VdpbIs9P/9+oE/lh0g5hXw+4Vf2ZW6lqQWAmuDceZa19R41q2TPqxl91fd8UXsq9dsxz3xI6 o0Hty1ZkKixwVPRZIPLWRpy754MSIYc0fDcvIv1vStgsoBHvh8su/hMLr4C4azwVnVZGfkeIivaTZ ceLwYu0EMHXJ+VKU3FCHATPHvGBygzC1sntIvWtc8lIeU0S0lNfCwUeoGt18JJQ6nUTYM28x4lqKD 8O8xb19IPZnPvIz6aobaRSBKULJBsbK+AEN0wvY5yRgRCY0gCTErONS7GoKKwdlzVImDd82VT5lYq Tx1LFR+Q==; Original-Received: from [10.9.9.212] (helo=mailfront12.runbox.com) by mailtransmit03.runbox with esmtp (Exim 4.86_2) (envelope-from ) id 1dwx2S-0005FI-IT for 25351@debbugs.gnu.org; Tue, 26 Sep 2017 23:05:08 +0200 Original-Received: from c-24-22-244-161.hsd1.wa.comcast.net ([24.22.244.161] helo=chinook) by mailfront12.runbox.com with esmtpsa (uid:179284 ) (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) id 1dwx1b-0000dt-5O for 25351@debbugs.gnu.org; Tue, 26 Sep 2017 23:04:15 +0200 In-Reply-To: (Gemini Lasswell's message of "Tue, 03 Jan 2017 19:06:17 -0800") X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 208.118.235.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" Xref: news.gmane.org gmane.emacs.bugs:137477 Archived-At: --=-=-= Content-Type: text/plain Here is a patch which fixes this bug by making a copy of the result of the first evaluation of a form to compare with the results of subsequent evaluations. This patch is based on the two patches I sent yesterday to bug#25316. Because Testcover may encounter circular structures in the code it instruments, I wrote a modified version of copy-tree which can handle circular structures. It's possibly generally useful enough to belong in subr.el alongside copy-tree instead of being an internal Testcover function, but I will submit that idea as a separate wishlist item. There isn't currently a version of `equal' which works for circular structures, so Testcover's handling of them is not fully implemented, but it now avoids stopping the code under coverage with an error message. --=-=-= Content-Type: text/plain Content-Disposition: attachment; filename=0001-Stop-Testcover-from-producing-spurious-1value-errors.patch >From 6545b936c2f458f6dbe437e6dec6acc4277d33fd Mon Sep 17 00:00:00 2001 From: Gemini Lasswell Date: Tue, 26 Sep 2017 08:14:23 -0700 Subject: [PATCH] Stop Testcover from producing spurious 1value errors (bug#25351) Fix bug#25351 by copying results of form evaluations for later comparison. * lisp/emacs-lisp/testcover.el (testcover-after): Copy the result of a form's first evaluation and compare subsequent evaluations to the copy. Improve the error message used when a form's value changes. (testcover--copy-object, testcover--copy-object1): New functions. * test/lisp/emacs-lisp/testcover-resources/testcases.el (by-value-vs-by-reference-bug-25351): Remove expected failure tag. (circular-lists-bug-24402): Add another circular list case. --- lisp/emacs-lisp/testcover.el | 95 ++++++++++++++++------ .../emacs-lisp/testcover-resources/testcases.el | 15 +++- 2 files changed, 81 insertions(+), 29 deletions(-) diff --git a/lisp/emacs-lisp/testcover.el b/lisp/emacs-lisp/testcover.el index d9c0d085e5..b9eca67427 100644 --- a/lisp/emacs-lisp/testcover.el +++ b/lisp/emacs-lisp/testcover.el @@ -49,11 +49,10 @@ ;; function being called is capable of returning in other cases. ;; Problems: -;; * To detect different values, we store the form's result in a vector and -;; compare the next result using `equal'. We don't copy the form's -;; result, so if caller alters it (`setcar', etc.) we'll think the next -;; call has the same value! Also, equal thinks two strings are the same -;; if they differ only in properties. +;; * `equal', which is used to compare the results of repeatedly executing +;; a form, has a couple of shortcomings. It considers strings to be the same +;; if they only differ in properties, and it raises an error when asked to +;; compare circular lists. ;; * Because we have only a "1value" class and no "always nil" class, we have ;; to treat as potentially 1-valued any `and' whose last term is 1-valued, ;; in case the last term is always nil. Example: @@ -259,26 +258,25 @@ testcover-after AFTER-INDEX is the form's index into the code-coverage vector. Return VALUE." (let ((old-result (aref testcover-vector after-index))) - (cond - ((eq 'unknown old-result) - (aset testcover-vector after-index value)) - ((eq 'maybe old-result) - (aset testcover-vector after-index 'ok-coverage)) - ((eq '1value old-result) - (aset testcover-vector after-index - (cons old-result value))) - ((and (eq (car-safe old-result) '1value) - (not (condition-case () - (equal (cdr old-result) value) - ;; TODO: Actually check circular lists for equality. - (circular-list t)))) - (error "Value of form marked with `1value' does vary: %s" value)) - ;; Test if a different result. - ((not (condition-case () - (equal value old-result) - ;; TODO: Actually check circular lists for equality. - (circular-list nil))) - (aset testcover-vector after-index 'ok-coverage)))) + (cond + ((eq 'unknown old-result) + (aset testcover-vector after-index (testcover--copy-object value))) + ((eq 'maybe old-result) + (aset testcover-vector after-index 'ok-coverage)) + ((eq '1value old-result) + (aset testcover-vector after-index + (cons old-result (testcover--copy-object value)))) + ((and (eq (car-safe old-result) '1value) + (not (condition-case () + (equal (cdr old-result) value) + (circular-list t)))) + (error "Value of form expected to be constant does vary, from %s to %s" + old-result value)) + ;; Test if a different result. + ((not (condition-case () + (equal value old-result) + (circular-list nil))) + (aset testcover-vector after-index 'ok-coverage)))) value) ;; Add these behaviors to Edebug. @@ -286,6 +284,53 @@ testcover-after (push '(testcover testcover-enter testcover-before testcover-after) edebug-behavior-alist)) +(defun testcover--copy-object (obj) + "Make a copy of OBJ. +If OBJ is a cons cell, copy both its car and its cdr. +Contrast to `copy-tree' which does the same but fails on circular +structures, and `copy-sequence', which copies only along the +cdrs. Copy vectors as well as conses." + (let ((ht (make-hash-table :test 'eq))) + (testcover--copy-object1 obj t ht))) + +(defun testcover--copy-object1 (obj vecp hash-table) + "Make a copy of OBJ, using a HASH-TABLE of objects already copied. +If OBJ is a cons cell, this recursively copies its car and +iteratively copies its cdr. When VECP is non-nil, copy +vectors as well as conses." + (if (and (atom obj) (or (not vecp) (not (vectorp obj)))) + obj + (let ((copy (gethash obj hash-table nil))) + (unless copy + (cond + ((consp obj) + (let* ((rest obj) current) + (setq copy (cons nil nil) + current copy) + (while + (progn + (puthash rest current hash-table) + (setf (car current) + (testcover--copy-object1 (car rest) vecp hash-table)) + (setq rest (cdr rest)) + (cond + ((atom rest) + (setf (cdr current) + (testcover--copy-object1 rest vecp hash-table)) + nil) + ((gethash rest hash-table nil) + (setf (cdr current) (gethash rest hash-table nil)) + nil) + (t (setq current + (setf (cdr current) (cons nil nil))))))))) + (t ; (and vecp (vectorp obj)) is true due to test in if above. + (setq copy (copy-sequence obj)) + (puthash obj copy hash-table) + (dotimes (i (length copy)) + (aset copy i + (testcover--copy-object1 (aref copy i) vecp hash-table)))))) + copy))) + ;;;========================================================================= ;;; Display the coverage data as color splotches on your code. ;;;========================================================================= diff --git a/test/lisp/emacs-lisp/testcover-resources/testcases.el b/test/lisp/emacs-lisp/testcover-resources/testcases.el index 61a457ac36..e4bc3fdb5a 100644 --- a/test/lisp/emacs-lisp/testcover-resources/testcases.el +++ b/test/lisp/emacs-lisp/testcover-resources/testcases.el @@ -357,7 +357,6 @@ testcover-testcase-baz ;; ==== by-value-vs-by-reference-bug-25351 ==== "An object created by a 1value expression may be modified by other code." -:expected-result :failed ;; ==== (defun testcover-testcase-ab () (list 'a 'b)) @@ -483,10 +482,18 @@ testcover-testcase-how-do-i-know-you "Testcover captures and ignores circular list errors." ;; ==== (defun testcover-testcase-cyc1 (a) - (let ((ls (make-list 10 a%%%))) - (nconc ls ls) - ls)) + (let ((ls (make-list 10 a%%%)%%%)) + (nconc ls%%% ls%%%) + ls)) ; The lack of a mark here is due to an ignored circular list error. (testcover-testcase-cyc1 1) (testcover-testcase-cyc1 1) +(defun testcover-testcase-cyc2 (a b) + (let ((ls1 (make-list 10 a%%%)%%%) + (ls2 (make-list 10 b))) + (nconc ls2 ls2) + (nconc ls1%%% ls2) + ls1)) +(testcover-testcase-cyc2 1 2) +(testcover-testcase-cyc2 1 4) ;; testcases.el ends here. -- 2.14.1 --=-=-=--