Warning about overwritten method definition

[gdalle]

I'm putting your package to the test in HiddenMarkovModels.jl, where the interface will look like this for version 0.2 (see the PR):

abstract type AbstractHiddenMarkovModel end
const AbstractHMM = AbstractHiddenMarkovModel

#  Docstrings omitted
Base.length
function initial_distribution end
function transition_matrix end
function obs_distribution end

@required AbstractHMM Base.length(::AbstractHMM)
@required AbstractHMM initial_distribution(::AbstractHMM)
@required AbstractHMM transition_matrix(::AbstractHMM)
@required AbstractHMM obs_distribution(::AbstractHMM, ::Integer)

The trouble is that I'm seeing some strange warnings when I first load my code:

julia> using HiddenMarkovModels
[ Info: Precompiling HiddenMarkovModels [84ca31d5-effc-45e0-bfda-5a68cd981f47]
WARNING: Method definition isInterface(Type{HiddenMarkovModels.AbstractHiddenMarkovModel}) in module HiddenMarkovModels at /home/guillaume/.julia/packages/RequiredInterfaces/FpEzi/src/RequiredInterfaces.jl:59 overwritten on the same line (check for duplicate calls to `include`).
  ** incremental compilation may be fatally broken for this module **

WARNING: Method definition getInterface(Type{HiddenMarkovModels.AbstractHiddenMarkovModel}) in module HiddenMarkovModels at /home/guillaume/.julia/packages/RequiredInterfaces/FpEzi/src/RequiredInterfaces.jl:60 overwritten on the same line (check for duplicate calls to `include`).
  ** incremental compilation may be fatally broken for this module **

[repeats several times]

Any clue where this behavior could come from?

[Seelengrab]

Ah, that's a silly mistake on my end - the @required macro is unconditionally redefining the internal methods that make this package work, resulting in the warnings. I'll push a fix & tag it in a few hours.

[Seelengrab]

Until then, it shouldn't lead to much worse than some invalidation in regards to those internal methods, so ought to be relatively benign.

[Seelengrab]

Ok, now that I had a look/think about this - this does turn out to be a bit more tricky, especially when taking working precompilation into account. The kinds of things I can do in a package while trying to give as clean an API as possible are limited, so I think the ultimate fix for this requires #1 as well, funnily enough.

The underlying issue is that this package "works" by abusing method definitions to lazily populate a global variable at first-use (it can't really be done in __init__ without moving the whole interface definition there, which seems super ugly). Trouble is, that redefines the function, possibly even before the method was called. By implementing #1, I can move the definition of the whole interface to the same block, meaning I can enforce generating all push!es here directly, no overriding required.

The downside of course is that it forces interface definitions to be at the same place - an issue that wouldn't exist if this were a language-level feature instead of a package. I can probably get around to implementing this tomorrow.

[gdalle]

The downside of course is that it forces interface definitions to be at the same place

What do you mean by at the same place? Like contiguous in the code? Even for multiple abstract types?

[gdalle]

I can probably get around to implementing this tomorrow.

Remember when I said no rush ^^? It's Sunday night around here, take a break :)

[Seelengrab]

What do you mean by at the same place? Like contiguous in the code? Even for multiple abstract types?

As in, all interface methods for one interface need to be in the same macro invocation :) So it'd end up with one @required block per interface. My hope was that this wasn't necessary, instead being able to incrementally build the interface, but seems like that's not quite possible.

Remember when I said no rush ^^? It's Sunday night around here, take a break :)

No worries, the fix isn't too big - and it'll have to wait until I find the time anyway :)

[Seelengrab]

@gdalle Can you try with the version from feat/multifunc_required? That branch implements #1 and fixes this issue, by disallowing multiple uses of @required on the same type entirely.

[gdalle]

It seems to work! And I might have found a way to include it in my package tests

[Seelengrab]

Ok, then I'll try to reproduce the 1.6 issues locally and tag a release afterwards.

As far as package tests go - https://github.com/JuliaLang/Pkg.jl/pull/3276 fixed this for me locally, so if PkgEval doesn't complain, it ought to work with the next release including that. Likely Julia 1.11?

[gdalle]

See https://github.com/Seelengrab/RequiredInterfaces.jl/issues/3#issuecomment-1629011102 for the workaround vis a vis 1.6