Implement `@deprecated` #22898

kristoff-it · 2025-02-15T19:08:46Z

Implements #22822

The part I'm most unsure about this implementation relates to how the build system represents -fallow-deprecated and -fno-allow-deprecated internally and how then this is reflected in the CLI invocation of build-exe and related subcommands.

because of allow-deprecated flags used by `@deprecated`

kristoff-it · 2025-02-15T19:10:19Z

lib/std/Build/Module.zig

@@ -284,6 +285,7 @@ pub fn init(
                .owner = owner,
                .root_source_file = if (options.root_source_file) |lp| lp.dupe(owner) else null,
                .import_table = .{},
+                .allow_deprecated = owner.graph.allow_deprecated orelse !owner.is_root,


In this version of the code modules track individually whether they're allowed to use deprecated code or not, giving to users the option of overriding the default behavior. See later comments for an alternative way of approaching this, although AFAIK it's not possible to make this system fully abuse-proof.

kristoff-it · 2025-02-15T19:11:30Z

lib/std/Build.zig

@@ -94,6 +94,9 @@ available_deps: AvailableDeps,

 release_mode: ReleaseMode,

+// True only for the top-level builder.
+is_root: bool = false,
+


As far as I'm aware whichever way this functionality is implemented requires tracking this information, which could be abused by users, for example by making the root builder forget that it's the root one.

I agree that while this could be abused, there's no way around it.

kristoff-it · 2025-02-15T19:16:30Z

lib/std/Build/Module.zig

@@ -557,6 +559,10 @@ pub fn appendZigProcessFlags(
    try addFlag(zig_args, m.pic, "-fPIC", "-fno-PIC");
    try addFlag(zig_args, m.red_zone, "-mred-zone", "-mno-red-zone");

+    if (m.root_source_file != null) {
+        try addFlag(zig_args, m.allow_deprecated, "-fallow-deprecated", "-fno-allow-deprecated");


If it's desirable to prevent individual modules in the Zig build system from overriding their allow-deprecated behavior, here we could instead check the is_root property of the builder that owns m.

As mentioned earlier this would not be a full solution though, since the root builder could be modified by the user.

I think this is a better idea. It's not so much that we want to stop people from sneakily changing the field; it's more that this is just a simpler way to do it (don't duplicate data needlessly!).

I definitely don't think the build system should support changing this flag on a per-module basis for now; assume YAGNI.

kristoff-it · 2025-02-15T19:17:52Z

src/main.zig

@@ -523,6 +523,8 @@ const usage_build_generic =
    \\  -fno-sanitize-thread      Disable Thread Sanitizer
    \\  -ffuzz                    Enable fuzz testing instrumentation
    \\  -fno-fuzz                 Disable fuzz testing instrumentation
+    \\  -fallow-deprecated        Allow usage of deprecated code
+    \\  -fno-allow-deprecated     Disallow usage of deprecated code


I've only added these flags for zig build (in the build runner) and here for modules, which means that the flags can't be used directly with zig build-exe or zig run.

I don't understand what you mean here. What you've changed here is the usage string and arg handling loop for build-exe, run, etc. Clearly you can pass this to build-exe; otherwise, the build system wouldn't be able to pass it. What did you mean by this?

I'm saying that zig build-exe foo.zig -fallow-deprecated doesn't work, because that's a module flag, not a global flag.

Maybe a global flag is desirable but then I'm not sure if it's ok for all of those to have the same name.

That's not how this works. If you never use -M, there's effectively an implicit -Mroot=the_source_file_you_passed at the end. If per-module flags didn't work with an implicit root module, then huge chunks of the compiler usage would be broken.

mlugg suggested a better way of implementing analysis of an istruction that cannot be referenced by other instructions.

andrewrk · 2025-02-17T19:59:11Z

Can't wait to use this immediately, like this:

    if (std.meta.hasMethod(T, "format")) {
        if (fmt.len > 0 and fmt[0] == 'f') {
            return value.format(fmt[1..], options, bw);
        } else {
            @deprecated();
            // After 0.14.0 is tagged, uncomment this next line:
            //@compileError("ambiguous format string; specify {f} to call format method, or {any} to skip it");
            //and then delete the `hasMethod` condition
            return value.format(fmt, options, bw);
        }
    }

kristoff-it · 2025-02-18T13:00:53Z

@andrewrk rewiew it and it's all yours :^)

mlugg · 2025-02-20T02:02:39Z

doc/langref.html.in

+      <pre>{#syntax#}@deprecated() void{#endsyntax#}</pre>
+      <p>
+	  Used to mark a given code path as deprecated. It evaluates to the same value
+	  passed in as argument, or the {#syntax#}void{#endsyntax#} value when given none.


Suggested change

passed in as argument, or the {#syntax#}void{#endsyntax#} value when given none.

passed in as argument, or to {#syntax#}{}{#endsyntax#} value when no argument is given.

Mainly the first bit; "the void value" reads like it might literally mean void

mlugg · 2025-02-20T02:05:02Z

doc/langref.html.in

+      As an example, in Zig 0.14.0 {#syntax#}std.time.sleep{#endsyntax#} was
+	  deprecated and the sleep function was moved to {#syntax#}std.Thread.sleep{#endsyntax#}.
+	  This is how this deprecation could have been expressed:
+
+      {#syntax_block|zig|lib/std/time.zig#}
+pub const sleep = @deprecated(std.Thread.sleep); // moved
+      {#end_syntax_block#}


The langref ought not to refer to specific old deprecations IMO. Perhaps:

Suggested change

As an example, in Zig 0.14.0 {#syntax#}std.time.sleep{#endsyntax#} was

deprecated and the sleep function was moved to {#syntax#}std.Thread.sleep{#endsyntax#}.

This is how this deprecation could have been expressed:

{#syntax_block|zig|lib/std/time.zig#}

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

{#end_syntax_block#}

For instance, consider a function which is moved to a different namespace, with a deprecated alias in the old namespace for compatibility. This is written as follows:

{#syntax_block|zig|old_namespace.zig#}

pub const my_function = @deprecated(new_namespace.my_function); // moved

{#end_syntax_block#}

Also, is it correct for syntax blocks to be inside a ? IIRC they should be outside of it, but I haven't checked.

mlugg · 2025-02-20T02:05:26Z

doc/langref.html.in

+      {#end_syntax_block#}
+	  </p>
+      <p>
+      By default it is a <b>compile error</b> to depend on deprecated code in


Suggested change

By default it is a compile error to depend on deprecated code in

By default it is a compile error to reference deprecated code in

"depend on" is vague

mlugg · 2025-02-20T02:07:00Z

doc/langref.html.in

+      </p>
+
+	  <p>
+      {#syntax#}@deprecated{#endsyntax#} can also be used without argument: 	


Suggested change

{#syntax#}@deprecated{#endsyntax#} can also be used without argument:

Using {#syntax#}@deprecated{#endsyntax#} without an argument can be useful inside of blocks:

We already know this usage exists; more helpfully, say why it exists!

lib/std/Build.zig

mlugg · 2025-02-20T02:13:06Z

lib/std/Build/Module.zig

@@ -557,6 +559,10 @@ pub fn appendZigProcessFlags(
    try addFlag(zig_args, m.pic, "-fPIC", "-fno-PIC");
    try addFlag(zig_args, m.red_zone, "-mred-zone", "-mno-red-zone");

+    if (m.root_source_file != null) {
+        try addFlag(zig_args, m.allow_deprecated, "-fallow-deprecated", "-fno-allow-deprecated");


I think this is a better idea. It's not so much that we want to stop people from sneakily changing the field; it's more that this is just a simpler way to do it (don't duplicate data needlessly!).

I definitely don't think the build system should support changing this flag on a per-module basis for now; assume YAGNI.

lib/std/zig/Zir.zig

mlugg · 2025-02-20T02:18:21Z

src/main.zig

@@ -523,6 +523,8 @@ const usage_build_generic =
    \\  -fno-sanitize-thread      Disable Thread Sanitizer
    \\  -ffuzz                    Enable fuzz testing instrumentation
    \\  -fno-fuzz                 Disable fuzz testing instrumentation
+    \\  -fallow-deprecated        Allow usage of deprecated code
+    \\  -fno-allow-deprecated     Disallow usage of deprecated code


I don't understand what you mean here. What you've changed here is the usage string and arg handling loop for build-exe, run, etc. Clearly you can pass this to build-exe; otherwise, the build system wouldn't be able to pass it. What did you mean by this?

mlugg · 2025-02-20T02:20:19Z

test/compile_errors.zig

+    {
+        const case = ctx.obj("usage of deprecated code", b.graph.host);
+
+        case.addError(
+            \\const bad = @deprecated(42);
+            \\
+            \\pub export fn foo() usize {
+            \\   return bad; 
+            \\}
+        , &[_][]const u8{
+            ":1:13: error: found deprecated code",
+        });
+    }


This file should have a big comment at the top saying "DON'T PUT THINGS HERE" :P

Please put this test in test/cases/compile_errors/deprecated.zig (or something like that) instead.

mlugg · 2025-02-20T02:26:36Z

test/tests.zig

+        // Test `zig build -fallow-deprecated`.
+
+        const deprecated_check: std.Build.Step.Run.StdIo.Check = .{
+            .expect_stderr_match = "found deprecated code",
+        };
+
+        const tmp_path = b.makeTempPath();
+
+        // create custom main.zig file containing a deprecated decl
+        {
+            const new_main_src =
+                \\const bad = @deprecated(42);
+                \\
+                \\pub fn main() u8 {
+                \\   return bad;
+                \\}
+                \\
+                \\test {
+                \\   if (bad != 42) return error.Bad;   
+                \\}
+            ;
+
+            var src_dir = std.fs.cwd().makeOpenPath(b.pathJoin(&.{ tmp_path, "src" }), .{}) catch unreachable;
+            defer src_dir.close();
+
+            var main = src_dir.createFile("main.zig", .{}) catch unreachable;
+            defer main.close();
+
+            main.writeAll(new_main_src) catch unreachable;
+        }


This isn't your fault, since you've just taken it from some of the surrounding cases, but... ooh, using std.fs in a build script is very naughty. I'm not gonna ask you to change this for now, though, since you're just following the surrounding cases.

However, these should be @panics, not unreachable; please do fix that.

kristoff-it added 4 commits February 13, 2025 20:47

initial implementation of @deprecated

fcf4784

stage1: update stage1.wasm

fd394d3

because of allow-deprecated flags used by `@deprecated`

@deprecated: add build system support

1464d21

@deprecated: add langref entry

b0b4d3a

kristoff-it commented Feb 15, 2025

View reviewed changes

kristoff-it force-pushed the deprecated-proposal branch from 378c387 to a46c2b2 Compare February 16, 2025 16:45

kristoff-it added 2 commits February 17, 2025 17:22

@deprecated: add tests

d5a332b

@deprecated: optimize sema implementation

a9d609a

mlugg suggested a better way of implementing analysis of an istruction that cannot be referenced by other instructions.

kristoff-it force-pushed the deprecated-proposal branch from a46c2b2 to a9d609a Compare February 17, 2025 16:38

mlugg requested changes Feb 20, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement `@deprecated` #22898

Implement `@deprecated` #22898

kristoff-it commented Feb 15, 2025

kristoff-it Feb 15, 2025 •

edited

Loading

kristoff-it Feb 15, 2025

mlugg Feb 20, 2025

kristoff-it Feb 15, 2025

mlugg Feb 20, 2025

kristoff-it Feb 15, 2025

mlugg Feb 20, 2025

kristoff-it Feb 21, 2025

mlugg Feb 22, 2025

andrewrk commented Feb 17, 2025

kristoff-it commented Feb 18, 2025

mlugg Feb 20, 2025

mlugg Feb 20, 2025

mlugg Feb 20, 2025

mlugg Feb 20, 2025

mlugg Feb 20, 2025

mlugg Feb 20, 2025

mlugg Feb 20, 2025

mlugg Feb 20, 2025

mlugg Feb 20, 2025

	passed in as argument, or the {#syntax#}void{#endsyntax#} value when given none.
	passed in as argument, or to {#syntax#}{}{#endsyntax#} value when no argument is given.

	By default it is a <b>compile error</b> to depend on deprecated code in
	By default it is a <b>compile error</b> to reference deprecated code in

	{#syntax#}@deprecated{#endsyntax#} can also be used without argument:
	Using {#syntax#}@deprecated{#endsyntax#} without an argument can be useful inside of blocks:

Implement @deprecated #22898

Are you sure you want to change the base?

Implement @deprecated #22898

Conversation

kristoff-it commented Feb 15, 2025

kristoff-it Feb 15, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrewrk commented Feb 17, 2025

kristoff-it commented Feb 18, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Implement `@deprecated` #22898

Implement `@deprecated` #22898

kristoff-it Feb 15, 2025 •

edited

Loading