Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

@deprecated() builtin #22822

Open
kristoff-it opened this issue Feb 9, 2025 · 15 comments
Open

@deprecated() builtin #22822

kristoff-it opened this issue Feb 9, 2025 · 15 comments
Labels
accepted This proposal is planned. proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Milestone

Comments

@kristoff-it
Copy link
Member

kristoff-it commented Feb 9, 2025

Motivation

It's already an established pattern in the Zig ecosystem to deprecate declarations first via a doc comment, and after a while by turning the old decl into a @compileError once the deprecation "grace period" is over, to then finally delete the decl altogether after some more time.

Currently a user that is interested in discovering early which APIs are being deprecated must grep the codebase for the word, find the decl the comment refers to, and figure out if and where its being used (directly or not) in the codebase.

Conversely, a library author that wants to maintain backwards compatibility (i.e. avoid a major version bump for a v1 package) will not want to set deprecated decls to @compileError as that would be a compatibility breakage, never giving users an opportunity to leverage compile errors to migrate their API usage.

It is possible today to implement a better solution entirely in userland:

// build.zig
fn build(b: *std.Build) void {
    const opts = b.addOptions();

    opts.addOption("deprecated", b.option(
        bool,
        "deprecated",
        "turn deprecated decls into compile errors",
    ) orelse false);
}
// root.zig
const options = @import("options");

fn oldFoo() void {
    if (options.deprecated) {
        @compileError("deprecated, use newFoo()");
    }
}

/// Deprecated: use `GenericWriter`
const Writer = if (options.deprecated) @compileError("deprecated, use `GenericWriter`") else GenericWriter;

Running zig build -Ddeprecated will turn deprecated decls in compile errors giving both a nice compiler-driven upgrade experience to users, while letting library authors maintain backwards compatibility.

This pattern seems so useful that might be worth enshrining it as a standardized practice.

Proposal

Introduce a @deprecated() builtin and a new -fdeprecated compiler flag to be used like so (this is a contrived example to showcase the workflow):

// main.zig
const std = @import("std");

const num = @deprecated(10);

pub fn main() void {
    std.debug.print("num: {}\n", .{num});
}
$ zig run main.zig
num: 10
$ zig run main.zig -fdeprecated

main.zig:3:13: error: found deprecated code
const num = @deprecated(10);
            ^~~~~~~~~~~~~~~
referenced by:
    main: main.zig:6:34
    posixCallMainAndExit: zig/lib/std/start.zig:647:22
    4 reference(s) hidden; use '-freference-trace=6' to see all references

A less contrived example would be the current deprecation of std.time.sleep in the Zig standard library:

/// Deprecated: moved to std.Thread.sleep
pub const sleep = std.Thread.sleep;
pub const sleep = @deprecated(std.Thread.sleep);

A second example: using @deprecated in a function body:

const Strategy = enum { greedy, expensive, fast };

fn compute(comptime strat: Strategy, comptime foo: bool, bar: usize) void {
    switch(strat) {
        .greedy => {
            // This strategy turned out to be bad when foo is false,
            // use the fast strategy instead.
            if (!foo) @deprecated();
            runGreedy(foo, bar);
        },
        .expensive => runExpensive(foo, bar),
        .fast => runFast(foo, bar),
    }
}

Lastly, since @deprecated conveys intent more precisely, tooling like LSPs and linters could offer diagnostics about deprecation automatically to the user in a more streamlined fashion than with the full userland solution.

A branch with @deprecated implemented can be found here: https://github.com/kristoff-it/zig/tree/deprecated-proposal (diff)

@RetroDev256
Copy link
Contributor

Can't wait for when we deprecate the @deprecated builtin ;)
Sounds like a smooth idea though, goes along with zig fmt fixups

@haze
Copy link
Contributor

haze commented Feb 9, 2025

Is there merit in requiring the author to provide a string or alternative symbol to steer users in the right direction? Is a comment on the same line enough?

@mlugg mlugg added the proposal This issue suggests modifications. If it also has the "accepted" label then it is planned. label Feb 9, 2025
@mlugg mlugg added this to the 0.15.0 milestone Feb 9, 2025
@nektro
Copy link
Contributor

nektro commented Feb 9, 2025

i think the rejection of #3320 and the demonstration of the userspace solution shows that this would fit much better as a standard library module or 3rd party package, than as a builtin

@kj4tmp
Copy link
Contributor

kj4tmp commented Feb 9, 2025

Pros:

  • supports maintainability goal of zig, deprecation is a key aspect of maintaining long-running projects and this would make it a "first-class" feature. Linting on "deprecated" in a doc comment is false-positive prone and this builtin will assist tooling.
  • supports only one obvious way to do things zen, this would be the only obvious way to mark deprecation
  • supports a use-case where a user wants to get a head start on preparing for deprecation / breaking API changes. Could argue this is not a compelling use case. If the code compiles and works, changes should not be made until upgrading the dependency is required for some other reason (security, features, etc). Could also argue that this is tech debt and we should support if possible its reduction quickly, early, and efficiently.
  • could prevent a user from wasting time implementing a solution that uses a deprecated API, only to have to change it later

Cons:

  • conversely also undermines only one obvious way to do things zen, user must decide if they will use -Ddeprecated or not.
  • (fundamentally new?) change to compile flags that would allow code to sometimes compile and sometime not (I'm thinking of warnings for C compilers here) (see rejection of Hacking-friendly "--sloppy" mode for Zig. #3320)
  • adds an additional failure mode that user must "hold in their head" when debugging issues with (yet to be created) maximum version selection: Version selection system for the official package manager #8284
  • -fno-deprecated is viral. As soon as my dependency starts having them and spamming my compiler output, I have to start ignoring them (or fix my dependency), but I suspect most will ignore them unless forced (see C compiler warnings!).

My conclusion:

Due to the virality of -fno-deprecated and the fundamental change to compile flags I think this should not be added. Maybe we add it but don't ever generate errors for it just so tooling/linters can reliably detect it.

@fluffinity
Copy link

i think the rejection of #3320 and the demonstration of the userspace solution shows that this would fit much better as a standard library module or 3rd party package, than as a builtin

So maybe implement this as a std function std.deprecrated? Getting this into std would at least mean the functionality does not need to be replicated across multiple projects.

Is there merit in requiring the author to provide a string or alternative symbol to steer users in the right direction? Is a comment on the same line enough?

It should definitely take a comptime string as an argument. This would make the message more explicit than a comment, which can easily get overlooked. Something like this should work:

pub fn depecrated(comptime message: []const u8, value: anytype) @TypeOf(value)

Due to the virality of -fno-deprecated and the fundamental change to compile flags I think this should not be added. Maybe we add it but don't ever generate errors for it just so tooling/linters can reliably detect it.

Maybe make this behavior configurable in the build.zig then? At least application authors should have the option to warn/error on deprecations. The problems you mention seems to primarily concern libraries. That being said, deprecation notices should be visible without external tooling, i.e the Zig compiler and ZLS should suffice to see it. Lets not become C/C++ in this regard.

@castholm
Copy link
Contributor

castholm commented Feb 9, 2025

As proposed, @deprecated seems like it would run into all the same problems that make optional warnings or --sloppy bad ideas in that all Zig code will get categorized as either "code that treats deprecations as errors" or "code that doesn't" and make it difficult for the former category to interact with the latter. Even the suggested user space implementation will have problems if you consider the case where I directly depend on and want package A with -Ddeprecated=true but I also depend on package B which wants package A with -Ddeprecated=false, resulting in package A not being deduplicated (which may or may not be a problem depending on how tightly integrated the packages are).

Speaking purely in the context of std, I believe the main problem people have with the current deprecation strategy is the lack of discoverability of deprecation notices, not the lack of compile errors. There's no good way as a user to get notified of a /// Deprecated doc comment unless you're diligently reading the documentation for all std APIs you interact with every time you update to a new Zig build. This leads to many projects only learning about deprecations when it's already too late and the affected APIs have already been removed or replaced, which is frustrating.

As much as I hate the term "code smell" it's also important to realize that using a deprecated API is a code smell, not a bug; a questionable pattern in your code that you should probably address, but not something that should necessarily break your build until you fix it. Any solution to the problem with status quo deprecations should focus on addressing the question "how can we let users know that they are using a deprecated API?" before it tries to go for "how do we prevent users from using a deprecated API?".

If there was some static analyzer or linter, either provided by a third-party or built into the zig CLI, that you could run (possibly as a separate step as part of your build process) to detect references from your own code to symbols documented with /// Deprecated and log them, it would solve the discoverability problem. Users could then run this tool/command after updating Zig and/or a dependency, get a list of deprecations and choose how, when and whether to address them.

@andrewrk
Copy link
Member

andrewrk commented Feb 11, 2025

Accepted, with adjustments:

  • flags are -fallow-deprecated and -fno-allow-deprecated.
  • Modules created by the root package default to -fno-allow-deprecated.
  • Modules created by dependency packages default to -fallow-deprecated.
  • Build system does not expose an option to set this flag.
  • zig build exposes -fallow-deprecated and -fno-allow-deprecated, applying it globally to entire dependency tree.

In other words, maintainers by default cannot make the mistake of calling into deprecated API, however, the dependency tree by default will not fail in "other people's code" due to deprecations.

To those who think this is the same thing as "sloppy mode": whereas sloppy mode would create two factions in the ecosystem who each prefer a different dialect of Zig, nobody wants to be using deprecated code. Programmers will naturally steer clear of any packages that require -fdeprecated in order to build, because it's an obvious clue that the package is of poor quality.

@andrewrk andrewrk added the accepted This proposal is planned. label Feb 11, 2025
@xdBronch
Copy link
Contributor

to clarify, you're saying the default should be that @deprecated() is an error? your wording (and zig philosophy :p) makes me assume yes but the way @kristoff-it wrote the proposal and implemented it is that -fdeprecated == an error is raised which seems to be the opposite of what you've said

@kristoff-it
Copy link
Member Author

Yes Andrew is changing the default behavior.

@kj4tmp
Copy link
Contributor

kj4tmp commented Feb 11, 2025

One pain point with deprecation for library authors is remembering when to turn a value into a compile error. Release schedules can be far apart, with deprecation being applied to many APIs over the course of months.

Should an additional parameter until be added? Making the signature:

fn deprecated(value: anytype, comptime until: []const u8) @TypeOf(value) {}

For example:

pub const sleep = @deprecated(std.Thread.sleep, "0.16.0");

Library authors would put the semantic version of when the value should be removed. And then when the build system attempts a build of the library with semantic version greater than or equal to "0.16.0" it would result in a compile error for the author.

Edit: No, this is already possible in user space with @compileError and branching on information from builtin

@castholm
Copy link
Contributor

The purpose of deprecation in Zig is therefore to help transition code authors to a new API. Due to version locking, errors due to deprecations will only occur when a maintainer upgrades a dependency to a new version.

How well will this play with #8284? Under MVS, if I directly depend on Package A version 1.4, but I also depend on Package B which in turn depends on Package A version 1.2, the build system will pick version 1.4 of Package A. If Package A has marked APIs with @deprecated between 1.2 and 1.4, and Package B uses those APIs, I am now forced to build with -fdeprecated, through no fault of my own. Is that correct?

(Semver specifies that "minor version [...] MUST be incremented if any public API functionality is marked as deprecated". Deprecations are not considered backward-incompatible and don't require a major version bump.)

Programmers will naturally steer clear of any packages that require -fdeprecated in order to build, because it's an obvious clue that the package is of poor quality.

For third-party libraries, this may be true, but for std and for as long as Zig remains pre-1.0, it will make it painful to depend on packages that target a "floating" Zig master version. Even if a package uses APIs that still exist and work, it will need to be updated much more frequently if it wants to avoid burdening consumers with requiring -fdeprecated, and if it wants to simultaneously target multiple Zig versions, it will need to litter the code with even more feature detection or explicit compiler version checks before using APIs that differ between versions.

At that point, the value of deprecation notices in std APIs is greatly diminished and you might as well stop doing them and instead always immediately break APIs by turning them into compile errors the moment they are removed, because both @deprecated and @compileError will put the same pressure on authors who want their packages to be considered "not poor".

@mlugg
Copy link
Member

mlugg commented Feb 11, 2025

Since the meaning of -fdeprecated isn't clear (Andrew flipped it and I could see either way; either "this lets you use deprecated code" or "this 'enables' @deprecated (so that it triggers errors)"), could I instead propose -fallow-deprecated and -fno-allow-deprecated?

@andrewrk
Copy link
Member

How well will this play with #8284?

Ah, now that's a delightfully productive question.

Note that this case still fits the pattern that the project built fine with -fno-allow-deprecated, and then the maintainer changes the dependency graph by adding a new package, and now there is a problem of deprecated API being used. This is equivalent to the existing problem of code being upgraded to a new version of a package and experiencing different behavior.

So it is still satisfying this use case:

The purpose of deprecation in Zig is therefore to help transition code authors to a new API. Due to version locking, errors due to deprecations will only occur when a maintainer upgrades a dependency to a new version. -fallow-deprecated is then a tool to help during the upgrade process, but the upgrade process will only be complete when the project compiles without the flag.

However, I'm going to backtrack a little bit, because it is still a bit of a problem in the sense that the purpose of deprecation, as pointed out via the semver quote, is to have a grace period in which the code continues to compile by default. The thing that makes me willing to backtrack here is considering the fact that the deprecated API can simply be removed when it is time for it to be a compile error, and the purpose of tooling-aware deprecation, then, is specifically for avoiding compilation errors while providing an upgrade path.

Anyway, here's the adjusted plan:

  • Modules created by the root package default to -fno-allow-deprecated.
  • Modules created by dependency packages default to -fallow-deprecated.

@onthebusiness
Copy link

In my mind, it would be better to have a type of compile error, like the logging levels we have in loggers.

@scheibo
Copy link
Contributor

scheibo commented Feb 12, 2025

In my mind, it would be better to have a type of compile error, like the logging levels we have in loggers.

Ah yes, lets call it a "warning" 🙃

To those who think this is the same thing as "sloppy mode": whereas sloppy mode would create two factions in the ecosystem who each prefer a different dialect of Zig, nobody wants to be using deprecated code.

To me this feels just like a distinction without a difference - why would I want to be using "sloppy" code either? Weakening Zig's stance on warnings specifically just for @deprecated feels silly to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted This proposal is planned. proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Projects
None yet
Development

No branches or pull requests