should aware top-level declarations even if it has not been referenced

[fuweid]

Zig Version

0.10.0-dev.3341+624633d7e

Steps to Reproduce

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

const ShouldFailEnum = enum(u2) {
    Zero,
    One,
    Two,
    Three,
    Four,    // should be failed
};

pub fn main() void {
    std.debug.print("Hello, {s}!\n", .{"World"});
}
$ zig run main.zig
Hello, World!

Expected Behavior

./main.zig:8:5: error: enumeration value 4 too large for type 'u2'
    Four,
    ^

Actual Behavior

It can be compiled successfully.

After check code(stage1), for the top-level declarations, compiler only cares exported or used var_function's symbols, even if the declarations are not valid, like fn oops() void { return 2022; }. I was trying to add other resolve_queue to do semantic_analyze, but I got a lot of errors. It is caused by deprecated public var_functions with @compileError , like https://github.com/ziglang/zig/blob/0.9.x/lib/std/log.zig#L227.

I am new to ziglang and not sure that it is bug. Just feel like that there should be warning or error for wrong code.

Thanks!

[wooster0]

This is working as intended and serves an important purpose in the language. For example comptime requires semantic analysis to be delayable in order to work properly. I think it's unlikely for this to change.

[fuweid]

This is working as intended and serves an important purpose in the language. For example comptime requires semantic analysis to be delayable in order to work properly. I think it's unlikely for this to change.

Thanks for the reply. Agree with that. The comptime requires there should not have any unused var or type. And unused type will not show in symbol table. But is it possible to have linter or something to check it?

[wooster0]

But is it possible to have linter or something to check it?

Not that I know of.

[InKryption]

If you want to enforce correctness within a given namespace, one thing you can do is make a test or comptime block which references all declarations:

comptime {
    // Could also choose to wrap this in `if (@import("builtin").is_test) {...}`,
    // so that it only gets evaluated if your running the file's tests, so that
    // normal compilation isn't bogged down by this.
    inline for (@typeInfo(@This()).Struct.decls) |decl| {
        _ = @field(@This(), decl.name);
    }
}

This will then force the compiler to evaluate all declarations in the enclosing namespace. There is also @import("std").testing.refAllDecls and @import("std").refAllDeclsRecursive, which effectively does the same thing, with the caveat being that they can't reference non-pub declarations.

My opinion: it isn't a very good idea to have this as a permanent fixture to refer to too great a number of declarations. At most, I think it's good:

1. as a one-off to check if there's any lingering nonsense, before committing changes.
2. in a namespace whose only job is to forward a bunch of other namespaces (like the @import("std") namespace).

[fuweid]

@InKryption Thanks！It is already helpful!

it isn't a very good idea to have this as a permanent fixture to refer to too great a number of declarations.

Agree. The package level public declarations should be handled in test. But the comptime script can be used for linter. LoL

[andrewrk]

Here is a related test case that is currently failing for stage2 but passing for stage1:

const ShouldFailEnum = enum(u2) {
    zero,
    one,
    two,
    three,
    four,
};
export fn foo() void {
    var x: ShouldFailEnum = .three;
    _ = x;
}

// error
// backend=stage2
// target=native
//
// :6:5: error: enumeration value 4 too large for type 'u2'

This case is not needed however because we already have compile error test case coverage for this; but it's currently in the "only-passing-for-stage1" category.

As for the current issue title, Zig is working as designed here. The enum integer type could be any arbitrary expression which could depend on conditional compilation. We don't find out what integer type the enum is until it is referenced. However, if the integer type is omitted, then the error could and should be reported eagerly in AstGen based on the number of items in the enum.