Fix Emacs byte-compile warnings in editor-integration/emacs#14798
Open
dannywillems wants to merge 15 commits into
Open
Fix Emacs byte-compile warnings in editor-integration/emacs#14798dannywillems wants to merge 15 commits into
dannywillems wants to merge 15 commits into
Conversation
The dune--field-vals macro expanded to a font-lock rule that referenced font-lock-constant-face as a variable. That variable is obsolete as of Emacs 31.1 and the byte-compiler warned about it twice (once per macro use site). Its value is the symbol font-lock-constant-face itself, so quoting the symbol produces an identical font-lock rule without the obsolete variable reference.
easy-menu-add has been a no-op in Emacs for a long time and is obsolete as of Emacs 28.1. The menu created by easy-menu-define is installed on its own, so the trailing easy-menu-add call can be removed without any change in behaviour. The function is called only for its side effect, so the changed return value is irrelevant.
dune-find-dominating calls xref-push-marker-stack, but xref is loaded lazily, so the byte-compiler reported the function as not known to be defined. Add a declare-function pointing at xref with the documented arglist. This is a compile-time hint only and does not change behaviour.
dune-smie-rules-verbose reads smie--parent, an internal SMIE variable bound dynamically during indentation. The byte-compiler treated it as a free variable. Add a value-less defvar after the smie require to mark it as a known special variable. This only affects compilation diagnostics, not runtime behaviour.
dune.el had no lexical-binding directive and defaulted to dynamic binding. The file does not rely on dynamic binding: the only closure (the project-p lambda in dune-root) captures its enclosing workspace binding, which works identically under lexical scoping. Adding the cookie silences the byte-compiler warning and introduces no new warnings. The file still loads and compiles cleanly.
dune-watch-filter-function never uses its process parameter; it is a process filter that operates on the captured watch-buffer. Rename the parameter to _process so the byte-compiler treats it as deliberately unused. The argument list keeps the same arity, so behaviour is unchanged.
dune-watch-on-kill-buffer reads dune-watch-minor-mode, the mode variable created by define-minor-mode later in the same file, so the byte-compiler reported it as a free variable. Add a value-less defvar before the first use to mark the symbol as special. The actual variable is still established by define-minor-mode, so behaviour is unchanged.
dune-flymake.el had only a coding cookie and defaulted to dynamic binding. All let-bound variables are local to their functions and no callee relies on them dynamically, so the file does not depend on dynamic binding. Add lexical-binding alongside the existing coding cookie. This silences the missing-directive warning and introduces no new warnings; the file still compiles with the same set of pre-existing free-variable diagnostics.
dune-flymake-dune-mode-hook pushes onto flymake-allowed-file-name-masks only when the newer flymake-proc-allowed-file-name-masks is unbound, but the byte-compiler reported the legacy name as a free variable in both the reference and the assignment. Add a value-less defvar so the compiler knows the symbol is special. The runtime boundp guard still selects the appropriate variable, so behaviour is unchanged.
Add a workflow that byte-compiles and loads dune.el, dune-flymake.el, and dune-watch.el on Emacs 29.1, 30.1, and snapshot. Emacs 29.1 is the effective floor because dune.el uses file-name-parent-directory, which was introduced in that release. The dependencies (flymake, smie, subr-x) are built in, so no extra packages are installed. Warnings are not treated as errors because a few behaviour-changing diagnostics remain (a compatibility fboundp guard, references to the undefined dune-program variable, and invalid defcustom :type values); fixing those would change behaviour and is left out of scope. The existing github-actions Dependabot config already covers this workflow.
Contributor
Author
|
This is an automated effort to help maintaining packages in the Emacs community. See https://x.com/dwillems42/status/2060720730338185699 and https://dannywillems.github.io/emacs-package-maintenance/ |
The dune-flymake-create-lint-script function checks and builds dune-flymake-program but then wrote to, chmod'd and invoked an undefined dune-program (a void variable at runtime). Reference dune-flymake-program throughout, which also clears the free-variable byte-compile warnings.
flymake-allowed-file-name-masks is obsolete as of Emacs 26.1 but is referenced only as a fallback for older Emacs; wrap the site in with-suppressed-warnings so the byte-compiler stays warning-clean on Emacs 26.1+ without dropping the legacy path.
Contributor
Author
|
Update: I have extended this to be byte-compile-clean with warnings-as-errors. It also corrects an undefined variable: dune-flymake.el referenced dune-program (never defined; void at runtime) where dune-flymake-program was intended. CI now byte-compiles with byte-compile-error-on-warn across Emacs 29.1, 30.1 and snapshot, all green. |
Collaborator
|
@dannywillems Can you fix the DCO on your commits? |
Collaborator
|
@shonfeder would you be able to take a look at this? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is an automated effort to help maintain Emacs packages in the
community by reducing byte-compile warnings.
The three Emacs Lisp files under
editor-integration/emacs/emittedseveral byte-compile warnings on recent Emacs (29.1, 30.1, snapshot).
This PR fixes every warning that can be addressed without changing
behaviour, one commit per warning type, and adds a CI workflow that
byte-compiles and loads the files on Emacs 29.1, 30.1, and snapshot.
Behaviour-preserving fixes
font-lock-constant-facevariable: quote the symbol inthe
dune--field-valsmacro so the generated font-lock rule no longerreferences the obsolete variable (identical rule, no behaviour change).
easy-menu-add: dropped the trailing no-op call indune-build-menu(the menu is installed byeasy-menu-define).xref-push-marker-stacknot known to be defined: added adeclare-functionfor it.smie--parent: forward-declared this internal SMIEvariable used by
dune-smie-rules-verbose.lexical-bindingcookie: added todune.elanddune-flymake.el(neither relies on dynamic binding; no new warnings).processparameter ofdune-watch-filter-functionto_process.dune-watch-minor-mode: forward-declared the modevariable (defined later in the same file by
define-minor-mode).flymake-allowed-file-name-masks: forward-declared thelegacy flymake variable used only on older Emacs.
CI
Adds
.github/workflows/emacs.ymlwhich byte-compiles and loads thethree files on Emacs 29.1, 30.1, and snapshot. Emacs 29.1 is the
effective floor because
dune.elusesfile-name-parent-directory(introduced in 29.1). Dependencies (flymake, smie, subr-x) are built in,
so no extra packages are installed. Warnings are not treated as errors
because a few behaviour-changing diagnostics remain (see below). The
existing github-actions Dependabot config already covers this workflow.
Residual warnings left out of scope (would change behaviour)
dune.el:value from call to fboundp is unusedfor the(fboundp (quote smie-prec2->grammar))guard. Removing the guard wouldchange behaviour on older Emacs lacking that function.
dune-flymake.el:reference to free variable dune-program(x2).dune-programis never defined; the intended variable is likelydune-flymake-program. Fixing it is a behavioural bug fix, out ofscope for warning hygiene.
dune-watch.el:bool is not a valid typein two defcustom forms(should be
boolean). This is a defcustom:typewidget change, whichalters the Customize interface, so it is left out of scope here.