bug#63251: AW: bug#63251: 28.2; vhdl-mode contribution

[Cyril Arnould <cyril.arnould@outlook.com>]

[-- Attachment #1: Type: text/plain, Size: 4564 bytes --]

> Why would they be set to a dotted list? Can you give an example?

Oh, sorry, I meant dotted pair, not list. So the (2 . 3) from the
ModelSim compiler entry, for example.

> I tried
>
>                               (cons :tag "Warning and Info"
>                                     (natnum :tag "Warning subexp index")
>                                     (natnum :tag "Info subexp index   "))
>
> instead of the (sexp ...) and it seems to work alright.

I agree, the above code seems to be the better option. I would
change the «Warning and Info» text though, seen as the
compilation command can differentiate between Warnings, Infos AND
Errors using the expression. Maybe «Type detection»? Or «Type
detection via regexp»?

> What I meant was that the code
>
>   (let ((tmp-alist vhdl-compiler-alist))
>     (while tmp-alist
>       (setcdr (nthcdr 3 (nth 11 (car tmp-alist)))
>               '(2 . nil))
>       (setq tmp-alist (cdr tmp-alist))))
>
> modifies the existing list structure instead of creating a new one based on the old. This can lead to surprises if parts of the structure being mutated is shared with structure elsewhere. Now this code probably does work, but it's a bit brittle, and it takes some work for the reader to understand that it's OK. Contrast it to something like (untested!)
>
>   (setq vhdl-compiler-alist
>         (mapcar (lambda (entry)
>                   ;; Add a `2' to the end of the list that is element #11.
>                   (append (take 11 entry)
>                           (append (nth 11 entry) (list 2))
>                           (nthcdr 12 entry)))
>                 vhdl-compiler-alist))
>
> where there is no mutation of the list structure, nor any sharing of a program constant whose accidental mutation might have very confusing consequences. (`take` is new in Emacs 29 but you can work around it by using `butlast` instead if the code needs to work with older Emacs versions.)

I have to admit this is going over my head a bit, but thanks for
explaining. I'll let it up to the maintainers to decide upon the
backwards compatibility code, I'm fine either way. Couldn't test
your changes though as I'm still on Emacs 28.2.


Von: Mattias Engdegård<mailto:mattias.engdegard@gmail.com>
Gesendet: Sonntag, 7. Mai 2023 18:22
An: Cyril Arnould<mailto:cyril.arnould@outlook.com>
Cc: 63251@debbugs.gnu.org<mailto:63251@debbugs.gnu.org>; Reto Zimmermann<mailto:reto@gnu.org>; Eli Zaretskii<mailto:eliz@gnu.org>
Betreff: Re: bug#63251: 28.2; vhdl-mode contribution

7 maj 2023 kl. 17.40 skrev Cyril Arnould <cyril.arnould@outlook.com>:

> I had tried (cons …) instead of (sexp …), but that just resulted in
> the customization menu breaking again if one of the compilers was set
> to a dotted list.

Why would they be set to a dotted list? Can you give an example?
I tried

                              (cons :tag "Warning and Info"
                                    (natnum :tag "Warning subexp index")
                                    (natnum :tag "Info subexp index   "))

instead of the (sexp ...) and it seems to work alright.

> > Think of what happens if later code performs an in-place change of that nil you added.
>
> I am by no means an expert when it comes to elisp, I don’t know what
> kind of problems this could cause.

What I meant was that the code

  (let ((tmp-alist vhdl-compiler-alist))
    (while tmp-alist
      (setcdr (nthcdr 3 (nth 11 (car tmp-alist)))
              '(2 . nil))
      (setq tmp-alist (cdr tmp-alist))))

modifies the existing list structure instead of creating a new one based on the old. This can lead to surprises if parts of the structure being mutated is shared with structure elsewhere. Now this code probably does work, but it's a bit brittle, and it takes some work for the reader to understand that it's OK. Contrast it to something like (untested!)

  (setq vhdl-compiler-alist
        (mapcar (lambda (entry)
                  ;; Add a `2' to the end of the list that is element #11.
                  (append (take 11 entry)
                          (append (nth 11 entry) (list 2))
                          (nthcdr 12 entry)))
                vhdl-compiler-alist))

where there is no mutation of the list structure, nor any sharing of a program constant whose accidental mutation might have very confusing consequences. (`take` is new in Emacs 29 but you can work around it by using `butlast` instead if the code needs to work with older Emacs versions.)



[-- Attachment #2: Type: text/html, Size: 10700 bytes --]