Definition reasoning of revert-buffer-function, should it be defvar-local?

[Daniel Nagy]

Hello,

I want to understand the reasoning behind the definition and the intention of
the variable `revert-buffer-function`.

In `files.el` I see the following definition:

>    (put 'revert-buffer-function 'permanent-local t)
>    (defvar revert-buffer-function #'revert-buffer--default)

The first line suggests to me, that the intention of the variable is that it is
supposed to be buffer-local, but `defvar-local` was not used. This means, that
if I open a new buffer and `(setq revert-buffer-function #'something)` then that
variable is assigned globally, breaking its behaviour. I know that I can use
`setq-local`, but using `defvar-local` would prevent accidents.

My question is, why was `defvar-local` not used? Should we change it ? Should we
change all these constructs where it says `(put ... 'permanent-local t)(defvar
...)`?

In `bookmark.el` I can also see the invocation `(setq revert-buffer-function
...)` but that instance does not leak out its value to global Emacs because its
mode is derived from `tabulated-list-mode` and that already uses `setq-local` to
make the value buffer-local.

Thank you already for your help.

For the more curious, this question sparked during this GitHub issue:
https://github.com/magit/magit/issues/4432

Best,
Daniel