From mboxrd@z Thu Jan  1 00:00:00 1970
Path: news.gmane.org!.POSTED.blaine.gmane.org!not-for-mail
From: Mattias =?UTF-8?Q?Engdeg=C3=A5rd?= <mattiase@acm.org>
Newsgroups: gmane.emacs.bugs
Subject: bug#36139: [PATCH] Make better use of the switch op in cond forms
Date: Wed, 19 Jun 2019 12:14:58 +0200
Message-ID: <E518FA46-88FB-456E-A91E-76758C7B1C13@acm.org>
References: <68467ACF-DA49-4EBA-BA3B-7339DB22A456@acm.org>
 <jwv8sty7luy.fsf-monnier+emacs@gnu.org>
Mime-Version: 1.0 (Mac OS X Mail 12.4 \(3445.104.11\))
Content-Type: text/plain;
	charset=us-ascii
Content-Transfer-Encoding: quoted-printable
Injection-Info: blaine.gmane.org; posting-host="blaine.gmane.org:195.159.176.226";
	logging-data="147612"; mail-complaints-to="usenet@blaine.gmane.org"
Cc: 36139@debbugs.gnu.org
To: Stefan Monnier <monnier@iro.umontreal.ca>
Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Wed Jun 19 12:32:56 2019
Return-path: <bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org>
Envelope-to: geb-bug-gnu-emacs@m.gmane.org
Original-Received: from lists.gnu.org ([209.51.188.17])
	by blaine.gmane.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256)
	(Exim 4.89)
	(envelope-from <bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org>)
	id 1hdXtc-000c4f-Eu
	for geb-bug-gnu-emacs@m.gmane.org; Wed, 19 Jun 2019 12:32:52 +0200
Original-Received: from localhost ([::1]:36642 helo=lists.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.86_2)
	(envelope-from <bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org>)
	id 1hdXtb-00055V-Fy
	for geb-bug-gnu-emacs@m.gmane.org; Wed, 19 Jun 2019 06:32:51 -0400
Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:56801)
 by lists.gnu.org with esmtp (Exim 4.86_2)
 (envelope-from <Debian-debbugs@debbugs.gnu.org>) id 1hdXsq-00040h-8Z
 for bug-gnu-emacs@gnu.org; Wed, 19 Jun 2019 06:32:06 -0400
Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
 (envelope-from <Debian-debbugs@debbugs.gnu.org>) id 1hdXdK-0002a9-HF
 for bug-gnu-emacs@gnu.org; Wed, 19 Jun 2019 06:16:03 -0400
Original-Received: from debbugs.gnu.org ([209.51.188.43]:60742)
 by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16)
 (Exim 4.71) (envelope-from <Debian-debbugs@debbugs.gnu.org>)
 id 1hdXdK-0002Zb-49
 for bug-gnu-emacs@gnu.org; Wed, 19 Jun 2019 06:16:02 -0400
Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2)
 (envelope-from <Debian-debbugs@debbugs.gnu.org>) id 1hdXdJ-0004YY-TO
 for bug-gnu-emacs@gnu.org; Wed, 19 Jun 2019 06:16:01 -0400
X-Loop: help-debbugs@gnu.org
Resent-From: Mattias =?UTF-8?Q?Engdeg=C3=A5rd?= <mattiase@acm.org>
Original-Sender: "Debbugs-submit" <debbugs-submit-bounces@debbugs.gnu.org>
Resent-CC: bug-gnu-emacs@gnu.org
Resent-Date: Wed, 19 Jun 2019 10:16:01 +0000
Resent-Message-ID: <handler.36139.B36139.156093930817424@debbugs.gnu.org>
Resent-Sender: help-debbugs@gnu.org
X-GNU-PR-Message: followup 36139
X-GNU-PR-Package: emacs
X-GNU-PR-Keywords: patch
Original-Received: via spool by 36139-submit@debbugs.gnu.org id=B36139.156093930817424
 (code B ref 36139); Wed, 19 Jun 2019 10:16:01 +0000
Original-Received: (at 36139) by debbugs.gnu.org; 19 Jun 2019 10:15:08 +0000
Original-Received: from localhost ([127.0.0.1]:46050 helo=debbugs.gnu.org)
 by debbugs.gnu.org with esmtp (Exim 4.84_2)
 (envelope-from <debbugs-submit-bounces@debbugs.gnu.org>)
 id 1hdXcS-0004Wx-2B
 for submit@debbugs.gnu.org; Wed, 19 Jun 2019 06:15:08 -0400
Original-Received: from mail153c50.megamailservers.eu ([91.136.10.163]:56866
 helo=mail50c50.megamailservers.eu)
 by debbugs.gnu.org with esmtp (Exim 4.84_2)
 (envelope-from <mattiase@acm.org>) id 1hdXcP-0004Wm-BY
 for 36139@debbugs.gnu.org; Wed, 19 Jun 2019 06:15:06 -0400
X-Authenticated-User: mattiase@bredband.net
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=megamailservers.eu;
 s=maildub; t=1560939303;
 bh=a1K51vvB6B0lt1kyljd75EBaQzYOqp5PZAfH7eJw74s=;
 h=Subject:From:In-Reply-To:Date:Cc:References:To:From;
 b=ME+LdwdBdrYVetEyhBhBsqkDQA9CVv2iFKSy1Moy9sqlSaf+hZ3HCu3wdAtNadgJx
 Egm/kQqkzrU8uco5WD3g4AAN2aH8ChBMTL6DrsTsdwEMEKRKln/t7q/9mGsJQovPc6
 wKA9x4ktAMEaX992stUnw5SJY9zxdzGb+W3bYxeI=
Feedback-ID: mattiase@acm.or
Original-Received: from [192.168.1.65] (c-e636e253.032-75-73746f71.bbcust.telenor.se
 [83.226.54.230]) (authenticated bits=0)
 by mail50c50.megamailservers.eu (8.14.9/8.13.1) with ESMTP id x5JAExFN002074; 
 Wed, 19 Jun 2019 10:15:02 +0000
In-Reply-To: <jwv8sty7luy.fsf-monnier+emacs@gnu.org>
X-Mailer: Apple Mail (2.3445.104.11)
X-CTCH-RefID: str=0001.0A0B0210.5D0A0B27.002F, ss=1, re=0.000, recu=0.000,
 reip=0.000, cl=1, cld=1, fgs=0
X-CTCH-VOD: Unknown
X-CTCH-Spam: Unknown
X-CTCH-Score: 0.000
X-CTCH-Flags: 0
X-CTCH-ScoreCust: 0.000
X-CSC: 0
X-CHA: v=2.3 cv=VLnzYeHX c=1 sm=1 tr=0 a=M+GU/qJco4WXjv8D6jB2IA==:117
 a=M+GU/qJco4WXjv8D6jB2IA==:17 a=jpOVt7BSZ2e4Z31A5e1TngXxSK0=:19
 a=kj9zAlcOel0A:10 a=iRZporoAAAAA:8 a=cfymPELf2vErCep-CmIA:9
 a=gNkfDvhx2r9dFsPu:21 a=PWl-oVwAyZQiZ0ev:21 a=CjuIK1q_8ugA:10
 a=NOBgFS-JBQ2l-kSd6-zu:22
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: 209.51.188.43
X-BeenThere: bug-gnu-emacs@gnu.org
List-Id: "Bug reports for GNU Emacs,
 the Swiss army knife of text editors" <bug-gnu-emacs.gnu.org>
List-Unsubscribe: <https://lists.gnu.org/mailman/options/bug-gnu-emacs>,
 <mailto:bug-gnu-emacs-request@gnu.org?subject=unsubscribe>
List-Archive: <https://lists.gnu.org/archive/html/bug-gnu-emacs>
List-Post: <mailto:bug-gnu-emacs@gnu.org>
List-Help: <mailto:bug-gnu-emacs-request@gnu.org?subject=help>
List-Subscribe: <https://lists.gnu.org/mailman/listinfo/bug-gnu-emacs>,
 <mailto:bug-gnu-emacs-request@gnu.org?subject=subscribe>
Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org
Original-Sender: "bug-gnu-emacs"
 <bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org>
Xref: news.gmane.org gmane.emacs.bugs:160838
Archived-At: <http://permalink.gmane.org/gmane.emacs.bugs/160838>

18 juni 2019 kl. 21.19 skrev Stefan Monnier <monnier@iro.umontreal.ca>:
>=20
>> A single `cond' form can how be compiled to any number of switch ops,
>> interspersed with non-switch conditions in arbitrary ways.
>=20
> It can also be compiled to a bunch of switch ops only, right?
> (e.g. if it starts with a switch on `x` and then is followed by
> a switch on `y`)

Correct, and good catch. I rephrased the commit message.

>> +    (and (> (length cases) 1)
>=20
> I think this `1` deserves a comment (IIRC it's the number of cases
> above which using a switch is expected to be faster than a sequence of
> tests).

Agreed, but the condition comes from the existing code =
(bytecomp.el:4180) where the number isn't motivated further either. I =
just assumed it was chosen with at least some care.

The ability to include multi-value cases in the switch makes the =
condition a conservative choice: if it is a good decision for =
single-value cases, it is definitely valid for multiple values per case.

I added a comment stating the intent, but it's not a lot more than =
restating the Lisp in English.

>> +        ;; Since `byte-compile-body' might increase =
`byte-compile-depth'
>> +        ;; by 1, not preserving its value will cause it to =
potentially
>> +        ;; increase by one for every clause body compiled, causing
>> +        ;; depth/tag conflicts or violating asserts down the road.
>> +        ;; To make sure `byte-compile-body' itself doesn't violate =
this,
>> +        ;; we use `cl-assert'.
>> +        (byte-compile-body body byte-compile--for-effect)
>> +        (cl-assert (or (=3D byte-compile-depth init-depth)
>> +                       (=3D byte-compile-depth (1+ init-depth))))
>=20
> IIRC the depth is altered depending on byte-compile--for-effect (if
> byte-compile--for-effect is non-nil when entering the function but nil
> afterwards, depth should be identical, and it should be increased by
> 1 otherwise), so we should be able to tighten this assertion to =
replace
> the `or` with an `if`.

I'll do that in a separate change then, because it seems to be =
orthogonal to my changes.
Brief experiments seem to indicate that the

        (byte-compile-body body byte-compile--for-effect)

call does not seem to alter byte-compile--for-effect, but it does =
increase depth by 1 iff byte-compile--for-effect is non-nil.

> Other than that, the patch looks fine to me.

Thanks for the review! Pushed to master.