Escape more narrowly in @message

[ettersi]

No description provided.

[ettersi]

Build failure is due to old Julia version (v1.6.7). This should work on Julia v1.10.0.

[mchitre]

Currently we define compat bounds for Fjage.jl as Julia 1.3. When was isexpr introduced? We will need to move the compat bounds if we are to merge this.

Two questions to think about:

1. Do we want to maintain our own version of @kwdef? Or is the escaping until Base.@kwdef is fixed acceptable?
2. Do we want to move up in Julia compat version? If so, what is the minimum Julia we want to support?

I don't have strong opinions on both, but would like us to think through this. @ettersi @notthetup thoughts?

[notthetup]

I support bumping up minimum Julia version. I don't think we're restricted on that side of things by anything.

[mchitre]

I support bumping up minimum Julia version. I don't think we're restricted on that side of things by anything.

Just to be sure that we're on the same page: We are talking about bumping minimum Julia version for all of Fjage.jl, not specific to UnetStack. And we are talking about bumping it to beyond the last LTS version, i.e., it will no longer work with LTS Julia.

[ettersi]

No harm having the discussion regarding minimum supported version, but isexpr is a one liner, so we don't need to upgrade for that. And I assume the new version of kwdef could go into compat.jl, so it could be available on older versions of julia as well. (I hadn't properly read up on Compat.jl before writing the previous sentence. Please ignore.)

[notthetup]

And we are talking about bumping it to beyond the last LTS version,

The last LTS is 1.6.7 right?

[mchitre]

No harm having the discussion regarding minimum supported version, but isexpr is a one liner, so we don't need to upgrade for that. And I assume the new version of kwdef could go into compat.jl, so it could be available on older versions of julia as well.

Maybe I am confused then. As per my understanding, your implementation of @kwdef uses Base.isexpr, which isn't available in last LTS (and hence the test fails), but is available in 1.10 (so works on latest Julia). Correct?

If so, we have 4 options:

1. Live with escaping the whole expression. Julia compat stays as is.
2. Add in our implementation of @kwdef as in this PR. But it depends on Base.isexpr, so update Julia compat to whatever version Base.isexpr was introduced in (LTS does not have it).
3. Add in our implementation of @kwdef as in this PR and add in a comat implementation of Base.isexpr if we are on older Julia. Julia compat stays as is.
4. Patch base Julia implementation of @kwdef and then depend on new Julia version. This is fine in long term, but not really an option for now.

[ettersi]

No harm having the discussion regarding minimum supported version, but isexpr is a one liner, so we don't need to upgrade for that. And I assume the new version of kwdef could go into compat.jl, so it could be available on older versions of julia as well.

Maybe I am confused then. As per my understanding, your implementation of @kwdef uses Base.isexpr, which isn't available in last LTS (and hence the test fails), but is available in 1.10 (so works on latest Julia). Correct?

That is correct, but isexpr is just this:

isexpr(@nospecialize(ex), head::Symbol) = isa(ex, Expr) && ex.head === head

So all problems around isexpr can easily be resolved either by copy-pasting the above line to Fjage.jl or manually inlining the function call.

From my point of view, the ideal solution would be this:

• The patch to @kwdef goes into Julia v1.10.1 (next patch release) and v1.6.8 (next LTS patch release). According to the Julia release process, that's the best we could possibly hope for.
• Fjage.jl adopts this branch as is for now, and then we delete kwdef.jl as soon as this patch is released on both the 1.10 and 1.6 branches.

Having said this, I can also see the logic behind @mchitre's Option 1:

• Live with escaping the whole expression. Julia compat stays as is.

I admit that as of now I can't think of a practically relevant situation where the solution currently implemented in the msg-no-eval branch would be a problem. So while the solution here may be slightly more "according to the book", the msg-no-eval branch is good enough and will likely require less maintenance in the foreseeable future.

---

Regarding minimum supported Julia version: Is there any reason for us to support anything before the current LTS version? As far as I'm aware, the only Julia versions we really need are the ones currently running in our modems. So currently that would be v1.7.1, and soon this will be replaced by v1.10?

[mchitre]

Fjage.jl is not just for modems, so we should look beyond the needs there. But last LTS and current stable is good enough IMO.

[ettersi]

But last LTS and current stable is good enough IMO.

... and the version running on the modems 😉

[mchitre]

LGTM. Happy to merge this in. We can only drop this much much later when LTS moves to a release after this gets merged in Julia base. So this stays in as part of fjåge.