[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#36139: [PATCH] Make better use of the switch op in cond forms
From: |
Mattias Engdegård |
Subject: |
bug#36139: [PATCH] Make better use of the switch op in cond forms |
Date: |
Wed, 19 Jun 2019 12:14:58 +0200 |
18 juni 2019 kl. 21.19 skrev Stefan Monnier <monnier@iro.umontreal.ca>:
>
>> A single `cond' form can how be compiled to any number of switch ops,
>> interspersed with non-switch conditions in arbitrary ways.
>
> 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)
>
> 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 (= byte-compile-depth init-depth)
>> + (= byte-compile-depth (1+ init-depth))))
>
> 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.
- bug#36139: [PATCH] Make better use of the switch op in cond forms, (continued)
- bug#36139: [PATCH] Make better use of the switch op in cond forms, Stefan Monnier, 2019/06/18
- bug#36139: [PATCH] Make better use of the switch op in cond forms, Stefan Monnier, 2019/06/18
- bug#36139: [PATCH] Make better use of the switch op in cond forms, Stefan Monnier, 2019/06/18
- bug#36139: [PATCH] Make better use of the switch op in cond forms, Stefan Monnier, 2019/06/18
- bug#36139: [PATCH] Make better use of the switch op in cond forms, Stefan Monnier, 2019/06/18
- bug#36139: [PATCH] Make better use of the switch op in cond forms,
Mattias Engdegård <=