fix: resolve Elixir 1.20.x compile-time warnings (#324) - #331
Merged
Conversation
Part of #324. - mix.exs: migrate `xref: [exclude: ...]` to the non-deprecated `elixirc_options: [no_warn_undefined: ...]`. - Evision.Mat.to_pointer/2: drop `opts || []`; the `is_list(opts)` guard already guarantees a list, so `|| []` was dead (`[]` is truthy). - Evision.Mat.to_nx/2: remove the `{:error, reason}` else clause the `with` chain can never produce (its failures are all tagged tuples). - Evision.Mat.check_unsupported_type/1: `true ->` in a `case` matched only the literal `true` and was already shadowed by the preceding `is_atom/1` clause, so a genuinely unsupported non-atom type raised CaseClauseError instead of the intended error. Use `_ ->`. - Evision.Zoo.Utils.HTTP: open the download target as a raw binary file and write with `:file.write/2`. IO.binwrite/2 is spec'd `:ok` in Elixir 1.20, so its `{:error, _}` branches were unreachable; raw writes return `:ok | {:error, posix}`, preserving disk-full handling (and are faster).
Part of #324. These py_src templates emit dead code that the Elixir 1.20 type checker flags; the generated modules are gitignored, so the fix belongs in the generator. - evision_extra_functions.py: drop `opts || []` in the CUDA GpuMat to_pointer/2 and from_pointer/4 templates. Both heads guard `is_list(opts)`, so opts is always a list and `|| []` was dead (`[]` is truthy). The auto-generated opts path is untouched, since its guard admits `nil` and still needs `|| []`. - fixes.py: remove the unreachable `_ -> raise` arm from the five `case bboxes.__struct__` blocks (nmsBoxes/nmsBoxesBatched/softNMSBoxes). Each function head already guards `is_struct(bboxes, Evision.Mat) or is_struct(bboxes, Nx.Tensor)`, so the two struct arms are exhaustive and the fallback could never match. Verified by applying the same transformations to the generated output and recompiling: the 7 warnings clear with no new (non-exhaustive) warnings.
…nerator Part of #324. The per-overload clause emitter deduplicated only by type signature, so two overloads that map to different types but subsuming Elixir guards each emitted a clause, which Elixir 1.20 flags as unreachable: FileStorage.write/3 and DNN.DictValue.dictValue/1 each have a `double` overload (is_number) followed by an `int` overload (is_integer) that is_number covers. Within a function's own overload group, skip a clause when an earlier same-arity clause subsumes it: guards equal position by position, or a bare `is_number/1` covering a later `is_integer/1` / `is_float/1`. FunctionVariant.normalized_guard builds the key with canonical positional argnames and one entry per positional arg (empty when unguarded), so signature length equals arity and clauses of different arity never subsume. `is_number` is matched exactly, so the Mat compound guard that merely contains it never triggers. Elixir only; Erlang generation is untouched. Validated by regenerating all 46 modules with and without the change: the diff removes only the two redundant `is_integer` clauses. Predicate covered by py_src/tests/test_guard_subsumes.py. The two GpuMat copy-constructor overloads that emit an identical `is_struct(_, Evision.CUDA.GpuMat)` clause come from separate overload groups and are left as-is; deduping across groups needs global state that risks dropping look-alike clauses elsewhere.
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
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.
Addresses #324. Fixes 15 of the 16 Elixir 1.20.x compile-time warnings in evision's own code (dependency warnings from nx/complex/kino/elixir_make are upstream and out of scope). The one remaining warning is called out below.
How the warnings were surfaced
EVISION_FETCH_PRECOMPILED=true mix compile --forcedrops the OpenCV build/download from the compiler chain (project config falls back to plainMix.compilers()), so the Elixir type checker runs overlib/alone.Changes (3 commits)
1. Hand-written modules (
2e544f0)mix.exs:xref: [exclude: ...]→elixirc_options: [no_warn_undefined: ...](former is deprecated).Evision.Mat.to_pointer/2: drop deadopts || [](theis_list(opts)guard already guarantees a list).Evision.Mat.to_nx/2: remove an{:error, reason}elseclause thewithchain can never produce.Evision.Mat.check_unsupported_type/1:true ->in acasematched only the literaltrueand was shadowed byis_atom/1, so unsupported non-atom types raisedCaseClauseError._ ->fixes both the warning and the latent bug.Evision.Zoo.Utils.HTTP:IO.binwrite/2is spec'd:okin 1.20, so its{:error, _}branches were unreachable. Switch to a raw binary file +:file.write/2(:ok | {:error, posix}), preserving disk-full handling and getting faster bulk writes.2. py_src templates (
99c1cac) — the emitted bindings are gitignored, so the fix lives in the generatorevision_extra_functions.py: drop deadopts || []in the CUDA GpuMatto_pointer/2/from_pointer/4templates. The auto-generated opts path is untouched (its guard admitsnil).fixes.py: remove the unreachable_ -> raisearm from the fivecase bboxes.__struct__blocks; each head already guardsis_struct(bboxes, Evision.Mat) or is_struct(bboxes, Nx.Tensor).3. Generator overload dedup (
35dbb54)The clause emitter deduplicated only by type signature, so overloads with different types but subsuming guards both emitted (
FileStorage.write/3,DNN.DictValue.dictValue/1: adouble/is_numberclause followed by a coveredint/is_integerclause). Added an argname-normalized, per-position guard signature and a conservative same-arity subsumption skip (guards equal, or bareis_numbercoveringis_integer/is_float).Validation (full regeneration)
Because the generator change touches all generated modules, I regenerated the bindings from the current OpenCV headers both with and without commit 3 (all 46 configured modules via
gen2.py) and diffed them. This caught two over-removal bugs that unit tests alone did not — one where an unguarded 2nd arg (e.g.Vec3f) collapsed a clause's signature length, and one where global dedup state leaked across unrelated overloads. Both are fixed; the final diff removes only the two redundantis_integerclauses, nothing else. The subsumption predicate has unit tests (py_src/tests/test_guard_subsumes.py, 14 cases incl. the regressions). The fullpy_srcsuite is green (2 pre-existing tests need a3rd_party/opencvcheckout).Net effect on the isolated recompile: redundancy/type warnings 10 → 1.
One remaining warning (not fixed here)
Evision.CUDA.GpuMat.gpuMat/1: two copy-constructor overloads emit an identicalis_struct(_, Evision.CUDA.GpuMat)clause, but from separate overload groups, so the per-function dedup above doesn't see them. Deduping across groups needs global state, which the regeneration diff showed drops reachable look-alike clauses elsewhere (e.g. infisheye) — so I left this one clause rather than ship a risky change. It's a pre-existing duplicate in the generated bindings and safe to leave; a targeted cross-group pass can handle it separately.The remaining
:evision_nif.*/1 is undefinedwarnings in the isolated compile are an artifact of building Elixir without the NIF; they don't appear in a real build.