bug#69749: (bindat-type sint 32 t) -> unbound variable bitlen

bug#69749: (bindat-type sint 32 t) -> unbound variable bitlen

[Eric Marsden]

-- file bd.el
(require 'bindat)
(bindat-pack (bindat-type sint 32 t) 42)
-- 
emacs -Q --batch -l bd.el -->

Error: void-variable (bitlen)
   mapbacktrace(#f(compiled-function (evald func args flags) #<bytecode 
-0x160fbb9816517825>))
   debug-early-backtrace()
   debug-early(error (void-variable bitlen))
   (/ bitlen 8)
   (+ bindat-idx (/ bitlen 8))
   (setq bindat-idx (+ bindat-idx (/ bitlen 8)))
   (let ((v field2)) (setq bindat-idx (+ bindat-idx (/ bitlen 8))))
   (let ((field2 (if (< v 0) (+ v wrap) v))) (let ((v field2)) (setq 
bindat-idx (+ bindat-idx (/ bitlen 8)))))
   (let ((v v)) (let ((field2 (if (< v 0) (+ v wrap) v))) (let ((v 
field2)) (setq bindat-idx (+ bindat-idx (/ bitlen 8))))))
   (lambda (v) (let ((v v)) (let ((field2 (if (< v 0) (+ v wrap) v))) 
(let ((v field2)) (setq bindat-idx (+ bindat-idx (/ bitlen 8)))))))(42)
   bindat--length-group(42 #s(bindat--type :le (lambda (v) (let ((v v)) 
(let ((field2 (if (< v 0) (+ v wrap) v))) (let ((v field2)) (setq 
bindat-idx (+ bindat-idx (/ bitlen 8))))))) :ue (lambda nil (let ((n (if 
t (bindat--unpack-uintr bitlen) (bindat--unpack-uint bitlen)))) (if (>= 
n max) (- n wrap) n))) :pe (lambda (v) (let ((v v)) (let ((field2 (if (< 
v 0) (+ v wrap) v))) (let ((v field2)) (if t (bindat--pack-uintr bitlen 
v) (bindat--pack-uint bitlen v))))))))
   bindat-length(#s(bindat--type :le (lambda (v) (let ((v v)) (let 
((field2 (if (< v 0) (+ v wrap) v))) (let ((v field2)) (setq bindat-idx 
(+ bindat-idx (/ bitlen 8))))))) :ue (lambda nil (let ((n (if t 
(bindat--unpack-uintr bitlen) (bindat--unpack-uint bitlen)))) (if (>= n 
max) (- n wrap) n))) :pe (lambda (v) (let ((v v)) (let ((field2 (if (< v 
0) (+ v wrap) v))) (let ((v field2)) (if t (bindat--pack-uintr bitlen v) 
(bindat--pack-uint bitlen v))))))) 42)
   bindat-pack(#s(bindat--type :le (lambda (v) (let ((v v)) (let 
((field2 (if (< v 0) (+ v wrap) v))) (let ((v field2)) (setq bindat-idx 
(+ bindat-idx (/ bitlen 8))))))) :ue (lambda nil (let ((n (if t 
(bindat--unpack-uintr bitlen) (bindat--unpack-uint bitlen)))) (if (>= n 
max) (- n wrap) n))) :pe (lambda (v) (let ((v v)) (let ((field2 (if (< v 
0) (+ v wrap) v))) (let ((v field2)) (if t (bindat--pack-uintr bitlen v) 
(bindat--pack-uint bitlen v))))))) 42)
   load-with-code-conversion("/tmp/bd.el" "/tmp/bd.el" nil t)
   command-line-1(("-l" "/tmp/bd.el"))
   command-line()
   normal-top-level()
Symbol’s value as variable is void: bitlen

This bug is difficult to understand. Run from an IELM buffer, this code 
works as expected. The macroexpansion of the bindat-type declaration 
includes a binding for bitlen.


GNU Emacs 29.2 (build 2, x86_64-pc-linux-gnu, GTK+ Version 3.24.41, 
cairo version 1.18.0) of 2024-03-09, modified by Debian

bug#69749: (bindat-type sint 32 t) -> unbound variable bitlen

[Eli Zaretskii]

> Date: Tue, 12 Mar 2024 16:18:58 +0100
> From: Eric Marsden <eric.marsden@risk-engineering.org>
> 
> -- file bd.el
> (require 'bindat)
> (bindat-pack (bindat-type sint 32 t) 42)
> -- 
> emacs -Q --batch -l bd.el -->
> 
> Error: void-variable (bitlen)
>    mapbacktrace(#f(compiled-function (evald func args flags) #<bytecode 
> -0x160fbb9816517825>))
>    debug-early-backtrace()
>    debug-early(error (void-variable bitlen))
>    (/ bitlen 8)
>    (+ bindat-idx (/ bitlen 8))
>    (setq bindat-idx (+ bindat-idx (/ bitlen 8)))
>    (let ((v field2)) (setq bindat-idx (+ bindat-idx (/ bitlen 8))))
>    (let ((field2 (if (< v 0) (+ v wrap) v))) (let ((v field2)) (setq 
> bindat-idx (+ bindat-idx (/ bitlen 8)))))
>    (let ((v v)) (let ((field2 (if (< v 0) (+ v wrap) v))) (let ((v 
> field2)) (setq bindat-idx (+ bindat-idx (/ bitlen 8))))))
>    (lambda (v) (let ((v v)) (let ((field2 (if (< v 0) (+ v wrap) v))) 
> (let ((v field2)) (setq bindat-idx (+ bindat-idx (/ bitlen 8)))))))(42)
>    bindat--length-group(42 #s(bindat--type :le (lambda (v) (let ((v v)) 
> (let ((field2 (if (< v 0) (+ v wrap) v))) (let ((v field2)) (setq 
> bindat-idx (+ bindat-idx (/ bitlen 8))))))) :ue (lambda nil (let ((n (if 
> t (bindat--unpack-uintr bitlen) (bindat--unpack-uint bitlen)))) (if (>= 
> n max) (- n wrap) n))) :pe (lambda (v) (let ((v v)) (let ((field2 (if (< 
> v 0) (+ v wrap) v))) (let ((v field2)) (if t (bindat--pack-uintr bitlen 
> v) (bindat--pack-uint bitlen v))))))))
>    bindat-length(#s(bindat--type :le (lambda (v) (let ((v v)) (let 
> ((field2 (if (< v 0) (+ v wrap) v))) (let ((v field2)) (setq bindat-idx 
> (+ bindat-idx (/ bitlen 8))))))) :ue (lambda nil (let ((n (if t 
> (bindat--unpack-uintr bitlen) (bindat--unpack-uint bitlen)))) (if (>= n 
> max) (- n wrap) n))) :pe (lambda (v) (let ((v v)) (let ((field2 (if (< v 
> 0) (+ v wrap) v))) (let ((v field2)) (if t (bindat--pack-uintr bitlen v) 
> (bindat--pack-uint bitlen v))))))) 42)
>    bindat-pack(#s(bindat--type :le (lambda (v) (let ((v v)) (let 
> ((field2 (if (< v 0) (+ v wrap) v))) (let ((v field2)) (setq bindat-idx 
> (+ bindat-idx (/ bitlen 8))))))) :ue (lambda nil (let ((n (if t 
> (bindat--unpack-uintr bitlen) (bindat--unpack-uint bitlen)))) (if (>= n 
> max) (- n wrap) n))) :pe (lambda (v) (let ((v v)) (let ((field2 (if (< v 
> 0) (+ v wrap) v))) (let ((v field2)) (if t (bindat--pack-uintr bitlen v) 
> (bindat--pack-uint bitlen v))))))) 42)
>    load-with-code-conversion("/tmp/bd.el" "/tmp/bd.el" nil t)
>    command-line-1(("-l" "/tmp/bd.el"))
>    command-line()
>    normal-top-level()
> Symbol’s value as variable is void: bitlen
> 
> This bug is difficult to understand. Run from an IELM buffer, this code 
> works as expected. The macroexpansion of the bindat-type declaration 
> includes a binding for bitlen.

Thanks, I added Stefan to this discussion.

bug#69749: (bindat-type sint 32 t) -> unbound variable bitlen

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>> -- file bd.el
>> (require 'bindat)
>> (bindat-pack (bindat-type sint 32 t) 42)
>> -- 

When you open this file in Emacs≥28, the modeline will have an orange
"/d" which is meant to warn you that something's amiss.
Click on it, and your problem will be solved :-)

>> Symbol’s value as variable is void: bitlen
>>
>> This bug is difficult to understand. Run from an IELM buffer, this code 
>> works as expected. The macroexpansion of the bindat-type declaration 
>> includes a binding for bitlen.

Your code ends up (via the `sint` of `bindat-type`) relying on the
`lexical-binding` dialect of ELisp, which is enabled by default in
`ielm` but not in your file.


        Stefan

bug#69749: (bindat-type sint 32 t) -> unbound variable bitlen

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>> Your code ends up (via the `sint` of `bindat-type`) relying on the
>> `lexical-binding` dialect of ELisp, which is enabled by default in
>> `ielm` but not in your file.
> Indeed, thanks for the fix.
> Perhaps it would be useful for the bindat-type macro to emit a warning when
> it is running in the dynamic-binding dialect (at least for the types that
> require lexical binding)?

Agreed.  I just pushed a corresponding patch to `master`.
It impacts only `sint` (among the predefined types).

> This would have no runtime cost when byte-compiled or native-compiled,
> I assume.

The patch I installed indeed doesn't impact the performance of the code.
The underlying detail which requires `lexical-binding` to be active does
cause the code for `sint` to be significantly less efficient than that
for `uint`, tho, so if performance is important, you may prefer to use
`uint` and do the conversion to a signed integer manually elsewhere. 🙁


        Stefan

bug#69749: (bindat-type sint 32 t) -> unbound variable bitlen

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Eli Zaretskii <eliz@gnu.org>,  69749@debbugs.gnu.org
> Date: Fri, 15 Mar 2024 12:51:36 -0400
> 
> >> Your code ends up (via the `sint` of `bindat-type`) relying on the
> >> `lexical-binding` dialect of ELisp, which is enabled by default in
> >> `ielm` but not in your file.
> > Indeed, thanks for the fix.
> > Perhaps it would be useful for the bindat-type macro to emit a warning when
> > it is running in the dynamic-binding dialect (at least for the types that
> > require lexical binding)?
> 
> Agreed.  I just pushed a corresponding patch to `master`.
> It impacts only `sint` (among the predefined types).
> 
> > This would have no runtime cost when byte-compiled or native-compiled,
> > I assume.
> 
> The patch I installed indeed doesn't impact the performance of the code.
> The underlying detail which requires `lexical-binding` to be active does
> cause the code for `sint` to be significantly less efficient than that
> for `uint`, tho, so if performance is important, you may prefer to use
> `uint` and do the conversion to a signed integer manually elsewhere. 🙁

Thanks, should we now close this bug?

bug#69749: (bindat-type sint 32 t) -> unbound variable bitlen

[Eli Zaretskii]

> Date: Sun, 17 Mar 2024 19:28:17 +0100
> Cc: 69749-done@debbugs.gnu.org
> From: Eric Marsden <eric.marsden@risk-engineering.org>
> 
> Le 3/16/2024 à 12:16 PM, Eli Zaretskii a écrit :
> >> The patch I installed indeed doesn't impact the performance of the code.
> >> The underlying detail which requires `lexical-binding` to be active does
> >> cause the code for `sint` to be significantly less efficient than that
> >> for `uint`, tho, so if performance is important, you may prefer to use
> >> `uint` and do the conversion to a signed integer manually elsewhere. 🙁
> > Thanks, should we now close this bug?
> 
> In my opinion yes, and thanks.
> 
> (Not sure whether my cc to -done will work.)

It will.