deprecation policy for Janet

[uvtc]

Would a deprecation policy be useful for Janet?

Does the Janet compiler currently have a mechanism for issuing deprecation warnings?

[uvtc]

I'm not very familiar with DLang, but looks like they have a 3-step deprecation process: https://dlang.org/deprecate.html

[bakpakin]

There is no mechanism for deprecation ATM. That said, we should probably start with a list of things we want to deprecate.

[uvtc]

Thanks.

we should probably start with a list of things we want to deprecate.

Other than the possibility of renaming s/reverse/reversed/ and s/reverse!/reverse/ (see #626), I don't yet know Janet well enough to offer suggestions on that.

[andrewchambers]

Another potential would be to shift pos? and friends to compare-pos? then make pos? work only on numbers again.

This sort of change must be a runtime check.

[pyrmont]

I like D's approach to having levels of deprecation. My suggestion for the levels would be:

D	Proposed	Meaning
spec	note	Use of the binding does not cause a warning (unless some 'extra warnings' setting is enabled).
dep	warn	Use of the binding will cause a warning (unless a 'no warnings' setting is enabled).
error	error	Use of the binding will cause an error.

I'd also note that earlier today @bakpakin merged #671 so in current Janet master it's now possible to use a struct in a binding declaration (e.g. def, defn, var, etc) to add arbitrary metadata. So, for example:

(defn old-and-busted {:deprecated :note} [x] (...))

If we did the above, flags (e.g. JANET_SLOT_DEP_NOTE, JANET_SLOT_DEP_WARN and JANET_SLOT_DEP_ERROR) could be added to the current JANET_SLOT_* flags in src/core/compile.h:

https://github.com/janet-lang/janet/blob/5c05dec65ae4b49e65b01f96984b81c8ca774ad4/src/core/compile.h#L76-L80

These flags could be used to mark a JanetSlot as containing a deprecated binding. When the compiler looks up a symbol, the compiler could then check whether the slot has one of these flags and emit a warning or an error.

[uvtc]

@pyrmont , in your example ("old-and-busted"), where would the docstring would go?

[pyrmont]

@uvtc The way that the Janet implementation of def and var work is by treating the first argument as the name, the last argument as the value and anything in between as an attribute. (In the case of defn, the attributes are the values between the name and the tuple that marks the argument list.) Attributes that are not of a supported type (i.e. a keyword, a string or a struct) result in an error.

All of this is to say that it doesn't really matter. The important thing is the type, not the position:

# one way
(defn old-and-busted {:deprecated :note} "This is a docstring" [x] (...))

# another way
(defn old-and-busted "This is a docstring" {:deprecated :note} [x] (..))

My preference would probably be to put the attributes between the name of the binding and the docstring but that's purely a personal thing.

Oh, and if you have attributes that use the same key, the last value wins.

# the value of the metadata associated with the`:foo` key is `:bar`, not `true`
(def x :foo {:foo :bar} 42)

# the value of the metadata associated with the `:foo` key is `true`, not `:bar`
(def x {:foo :bar} :foo 42)

[bakpakin]

The deprecation note policy is good, and is a good direction to go for non-error compiler feedback. Currently, the compiler will only notify you that anything is happening if a macro is executed with side effects.

[bakpakin]

There is a recent branch https://github.com/janet-lang/janet/tree/linting that is the beginning of this, as well as a more general mechanism for compiler warnings.

[bakpakin]

So to update further, 1.16.0 has been released with these linting additions as well as a general mechanism for linting warnings. There are three lint levels, :relaxed, :normal, and :strict, where :normal is the default. :strict deprecation should indicate deprecation issues that will only show up if the user has nit-picky preferences set, while :relaxed indicates that the deprecation will show up even if the user has :relaxed deprecations set.

(defn my-old-function
  "Some old function that people should stop using"
  :deprecated
  [x]
  (+ x x))

(defn my-old-function-2
  "Another old function. Really don't use this if you can avoid it."
  {:deprecated :relaxed}
  [x y z]
  (+ x x x y y z))

(def binding :deprecated "also works for defs, vars, and macros." 10)

# All cause compiler warnings
my-old-function # yes, even just referencing it triggers the warning.
(my-old-function-2 1 2 3)
(+ 1 binding)

Each lint message has a lint level, which can either be a warning or an error depending on some external settings - this logic is all determined in run-context - the compiler itself will just emit an array of lint issues.

Each lint message has the following structure:

(level line column message)

Where level is a keyword lint level, line and column are the line and column of the source code where the lint issue occurred, and message is a user facing message. The deprecation system is based on this, and setting different deprecation levels will just emit those lint levels.

Also want to point out that the linting mechanism is extensible in macros so users can write their own compiler warnings. Inside a macro, the linting array is exposed as (dyn :macro-lints), so you can append your own linting errors inside macros for interesting effects.

[pyrmont]

@bakpakin I was testing this out with Janet 1.16.0 but am not sure if I'm seeing the correct behaviour (or if I'm using it correctly).

When I'm in the REPL, if I write:

(def normal-binding :deprecated 10)
(def relaxed-binding :deprecated :relaxed 10)
(def strict-binding :deprecated :strict 10)

Then I see the warning message whenever I use any of the above bindings. This was a little surprising since I assumed the :strict warning wouldn't appear but perhaps :strict checking is on by default for REPLs. However, if I then create a file with the above bindings and run it at the command line, I get no compiler warnings at all. What should I be doing?

[pyrmont]

@bakpakin Apologies, I realised I wasn't actually using the bindings so hence no warnings. I see the warnings now.

I am still unsure how the levels are intended to be manipulated. Is there some documentation on that?

[bakpakin]

My bad with the examples, it should be (def x {:deprecated :strict} 10). The surrounding struct is needed

[bakpakin]

This uses the normal mechanism for adding metadata to top level bindings.

[pyrmont]

@bakpakin Thanks for the clarification. Confirm that in Janet 1.16.0 when I have a file, test.janet, with the following:

(def x :deprecated 1)

(inc x)

(def y {:deprecated :strict} 1)

(inc y)

and execute it with janet test.janet I only see a compiler warning for the x binding and not the y binding.

[sogaiu]

@bakpakin Are you thinking to expose some command line option?

[pepe]

@sogaiu it seems, it is already done https://github.com/janet-lang/janet/commit/4b96b7385896c8278364a0ac52a5efd3b0704b10 and it seems it is also possible with the dyns :lint-error :lint-warn, which I need for the jlnt.kak :-).

[pepe]

And it also seems, there are more goodies like getting the offending line on compile errs and warns. Yay!

[sogaiu]

@pepe Thanks!

[kenaryn]

Could the keyword-style argument be a candidate for deprecation as the named argument is said in the doc to be more convenient? I admit the latter lacks orthogonality due to a lack of invalidating wrong parameters' format.