Skip to content

fix: resolve byte-compile warnings without changing behaviour#1242

Open
dannywillems wants to merge 12 commits into
Fuco1:masterfrom
dannywillems:fix/byte-compile-warnings
Open

fix: resolve byte-compile warnings without changing behaviour#1242
dannywillems wants to merge 12 commits into
Fuco1:masterfrom
dannywillems:fix/byte-compile-warnings

Conversation

@dannywillems
Copy link
Copy Markdown

This is an effort to reduce byte-compile warnings in smartparens
without changing any runtime behaviour. Each warning type is fixed in
its own commit, and a separate commit updates CI.

All source files were byte-compiled with byte-compile-warnings t
(Emacs 30.1 and the snapshot) before and after each change to confirm
the targeted warnings disappear and no new warnings appear. The
package still loads and the test suite behaviour is unchanged.

Behaviour-preserving fixes

  • Obsolete lsh: replaced lsh with ash in the syntax-flag bit
    masks. Every call site is a left shift of the positive value 1 by
    a fixed non-negative count (16 to 20), where lsh and ash are
    equivalent.
  • Docstring unescaped single quotes: escaped the literal single
    quotes in syntax-flag descriptions, a quoted symbol, and the Python
    dictionary examples with \=' so they render verbatim.
  • Obsolete if-let / when-let: rewrote the affected forms using
    a plain let binding followed by if or when, avoiding both the
    obsolete macros and the if-let* / when-let* variants, keeping
    compatibility with the existing minimum Emacs version.
  • Obsolete point-at-eol: replaced with line-end-position,
    which is an exact long-standing equivalent.
  • Unused lexical bindings: underscored the dummy hook arguments of
    sp--reset-memoization and sp--strict-regexp-opt (arity kept),
    and dropped the write-only paired-delim, the unread next-char-fn
    in the sp--skip-to-symbol-1 macro, and the unused :next-thing
    destructuring in sp-lisp-insert-space-after-slurp.
  • Docstrings wider than 80 characters: reflowed two docstrings.
  • Free variables and undefined functions: added forward defvar
    declarations for smartparens-strict-mode (defined later via
    define-minor-mode but referenced earlier) and ess-roxy-re, and
    declare-function forms for the org functions used inside the
    eval-after-load 'org block in smartparens-config.

CI

Added a dedicated byte-compile job that installs dash via
package.el, byte-compiles every source file, and loads the package.
This makes the compile result independent of the test suite. The
matrix now also includes Emacs 30.1. The workflow runs on
pull_request as well, uses actions/checkout@v5, and a
dependabot.yml keeps the GitHub Actions up to date.

Residual warnings left out of scope (behaviour-changing)

These were intentionally left unchanged because fixing them would
alter behaviour or load semantics:

  • defadvice is obsolete as of Emacs 30.1 (in smartparens.el,
    smartparens-ess.el, smartparens-python.el). Migrating to
    advice-add / define-advice changes activation/load semantics.
  • cycle-spacing called with 3 arguments in smartparens-ess.el.
    The extra arguments are passed for compatibility with older Emacs;
    changing the arity is a behaviour change.
  • value from call to <unused> in sp--get-active-overlay. This is
    an artifact of the dash --reduce macro expansion (the < result
    is used as the if condition); it is not fixable inside smartparens
    without restructuring the call.

Because byte-compilation cannot be made warning-free without changing
behaviour, the new CI job does not use -Werror; it instead requires
that the sources byte-compile and load successfully.

The byte-compiler warns to avoid lsh in favour of ash. All affected
call sites perform left shifts of the positive value 1 by a fixed
non-negative count (16 to 20) to build syntax-flag bit masks, where
lsh and ash are equivalent. The replacement preserves behaviour.
The byte-compiler warns about unescaped single quotes in docstrings,
which it would otherwise render as curly quotes. Escape the literal
single quotes used in syntax-flag descriptions, a quoted symbol, and
Python dictionary examples with \=' so they render verbatim. No
behaviour change.
The if-let and when-let macros are marked obsolete by the byte
compiler. Rewrite the affected forms using a plain let binding
followed by if or when. This avoids the obsolete macros without
introducing the if-let*/when-let* variants, keeping compatibility
with the existing minimum Emacs version. No behaviour change.
point-at-eol is an obsolete function alias for line-end-position
since Emacs 29.1. line-end-position has been available far longer,
so the replacement preserves behaviour and the minimum supported
Emacs version.
Silence unused-variable warnings without changing behaviour:

- Prefix the dummy hook arguments of sp--reset-memoization and
  sp--strict-regexp-opt with an underscore, keeping their arity.
- Drop the write-only paired-delim binding in sp-get-expression and
  the unread next-char-fn binding in the sp--skip-to-symbol-1 macro.
- Drop the unused :next-thing destructuring in
  sp-lisp-insert-space-after-slurp.
Two docstrings exceeded the 80-column limit flagged by the byte
compiler. Reflow them across multiple lines without changing the
documented behaviour.
Resolve free-variable and undefined-function warnings without
changing behaviour:

- Add a forward defvar for smartparens-strict-mode, which is defined
  later via define-minor-mode but referenced in smartparens-mode.
- Add a forward defvar for ess-roxy-re, alongside the existing
  ess-roxy-str declaration.
- Add declare-function forms for the org functions used inside the
  eval-after-load org block in smartparens-config.
Add a dedicated byte-compile job so the compilation result is
independent of the test suite. The job installs the only hard
dependency (dash) via package.el, byte-compiles every source file,
and loads the package to confirm it works.

The sources cannot be made fully warning-free without changing
behaviour, so byte-compilation does not use -Werror: a dash --reduce
expansion triggers a spurious value-unused warning on every Emacs
version, and Emacs 30.1+ flags obsolete defadvice and cycle-spacing
arity. The matrix adds Emacs 30.1 and keeps snapshot.

Also run on pull_request, bump actions/checkout to v5, and add a
dependabot config to keep the GitHub Actions up to date.
@dannywillems
Copy link
Copy Markdown
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 shortest-overlay selection used a dash `--reduce' whose macro
expansion makes the byte-compiler report "value from call to '<' is
unused" even though the comparison result is used. Replace it with an
equivalent `--min-by' that keeps the earliest overlay on ties, which
reproduces the previous selection exactly without the warning.
`cycle-spacing' lost its PRESERVE-NL-BACK and MODE arguments in Emacs
29.1, so the 3-arg calls trigger an arity warning on Emacs 29.1 and
later. In every pre-29 Emacs `just-one-space' was defined as
`(cycle-spacing N nil 'single-shot)', so these calls are exactly
equivalent. The single MODE=t call runs as a post-handler (never a
repeated identical command), so it executes the same first step as
single-shot. Behaviour is unchanged on all supported Emacs versions.
`defadvice' became an obsolete macro in Emacs 30.1, producing a
warning for every legacy advice in the package. These advices rely on
`ad-do-it', `ad-get-arg' and `ad-return-value' to wrap the original
commands; porting them to `advice-add' is a separate behaviour-
affecting change. Wrap each definition site in
`with-suppressed-warnings ((obsolete defadvice) ...)' to silence only
the obsolete-macro warning while keeping the existing advice mechanism.
`with-suppressed-warnings' is available since Emacs 27.1, the project
minimum.
Set byte-compile-error-on-warn so the byte-compile job fails on any
warning. The sources are now warning-free on every Emacs version in the
matrix (27.1 through 30.1 and snapshot), so -Werror can be applied
uniformly without masking anything.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant