ziglang / zig

General-purpose programming language and toolchain for maintaining robust, optimal, and reusable software.
https://ziglang.org
MIT License
34.03k stars 2.49k forks source link

Problems arising from conflict between names of `std.builtin` and `@import("builtin")` #19690

Open SeanTheGleaming opened 5 months ago

SeanTheGleaming commented 5 months ago

Problem

A common pattern when making an import is to make a top level declaration importing that module, and then make top level declarations for the namespaces of that module which you use. For example:

const std = @import("std");
const math = std.math;
const os = std.os;
...

This does create a small issue though. Consider the a use case where I will be using both the std.builtin namespace and the @import("builtin") module. This creates a conflict when using the widely accepted pattern for imports:

// By the usual convention, we would have two decls named builtin
const std = @import("std");
const builtin = std.builtin;
const builtin = @import("builtin");

This would normally be pretty innocuous, but the problem lies in that for real-world cases where @import("builtin") is used, it is usually used in tandem with std.builtin, since the two deal with often-intersecting use cases. For example, consider a use case where I want to do metaprogramming with std.builtin.Type, but also want to check whether libc is linked via @import("builtin").link_libc, or if I want to access @import("builtin").target and also use std.builtin.panicOutOfBounds.

Proposed Solutions

1. Rename std.builtin (Or maybe rename @import("builtin")

This is a simple enough solution, but needless to say, this would be a breaking change which would affect many projects. You could also apply the same idea and rename the @import("builtin") module, which would have similar implications. On top of fixing the problem, another benefit of this solution is that, at least in my opinion, "builtin" is a bit of a misleading name for both the module and the std namespace. A new name may make their purposes more clear. For example, @import("builtin") could be changed to @import("env") to more clearly indicate that it contains declarations regarding the environment which the code is being compiled under

2. Mix @import("builtin") into std.builtin

This is the solution I am most inclined to implementing. All it would require is adding the line pub usingnamespace @import("builtin"); into lib/std/builtin.zig. This would eliminate the need to use @import("builtin") outside of the standard library, reducing the earlier example to the following while breaking no existing code

const std = @import("std");
const builtin = std.builtin;

All it would require is adding the line pub usingnamespace @import("builtin"); into lib/std/builtin.zig. And if we don't want a usingnamespace statement, it would be easy enough to just manually declare each value with how small @import("builtin") is. So the two ways to do this would are:

1.

<in lib/std/builtin.zig>
pub usingnamespace @import("builtin");

2.

<in lib/std/builtin.zig>
pub const abi = @import("builtin").abi;
pub const code_model = @import("builtin").code_model;
pub const cpu = @import("builtin").cpu;
...
mlugg commented 5 months ago

The mixin approach is not viable because the contents of @import("builtin.zig") may differ between modules in one Zig compilation. And yes, for the record, there is a hard ban on introducing uses of usingnamespace into the standard library for now.

ni-vzavalishin commented 5 months ago

I have no idea how common is this approach:

const std = @import("std");
const math = std.math;
const os = std.os;

but I don't find this pattern convincing. I'd argue that, as a general rule, writing subspaces of std out explicitly has a positive impact on the readability of the code.

castholm commented 5 months ago

Anecdote from the point of view of a user: I started writing Zig about a year and a half ago and I distinctly remember that for the week or so I found the term "builtin" very overloaded and confusing, repeatedly mistaking @import("std").builtin for @import("builtin") until I eventually found my bearings.

The name std.builtin makes sense from the point of view that it defines and exposes interfaces that support @-prefixed built-in functions.

The generated builtin module that provides information about the compiler and target, however, I think has a misleading name. Aside from the fact that it's always automatically generated by the compiler, there is nothing especially "built-in" about it, and it is functionally more similar to something like std.Build.Options than the rule-breaking "you can't implement this in user space" functionality usually associated with the term. The documentation refers to builtin as "compile variables", so I wonder if using a different name like compile_vars, build_env, comptime_env, compilation, etc. would help convey its purpose more clearly and make it less likely to get mixed up with std.builtin.

Lking03x commented 5 months ago

Slightly renaming is preferable.

  1. I propose add a 's' to one of them

    const builtin = @import("builtin");
    const builtins = @import("std").builtins;
  2. Rename @import("builtin"); to @import("builtin_info"); or @import("builtin_constants");

We can do both 1. and 2. above.

deanveloper commented 3 months ago

Personally, I think they still deserve separate names since they contain separate things -- @import("build") contains constants related to the build configuration, while @import("std").builtin contains types/variables/functions provided by the language. (Naming mentioned... it's bikeshedding time!)

The name "builtin" also becomes confusing when you consider that Zig also has a concept of "builtin functions" (the @-prefixed functions)

I propose @import("builtin") to be renamed to @import("build"), and @import("std").builtin to be renamed to @import("std").zig. I think the @-functions should continue being called "builtin functions". I think these three names effectively communicate what they are pretty well!

const build = @import("build");
const zig = @import("std").zig;

// some common usages of both of these namespaces

if (build.mode == zig.OptimizeMode.Debug) { ... } // btw, we should probably fix the casing here...

if (build.is_test) { ... }

pub fn Wrapper(comptime T: type) type {
    return @Type(zig.Type.Struct{ ... });
}
nektro commented 3 months ago

the general solution is to not do const builtin = std.builtin;. fully qualifying your names makes for much more readable code.

deanveloper commented 3 months ago

I agree, typically I don't actually do stuff like const builtin = std.builtin; (except for namespaces in my own code sometimes), was just doing that for the example I guess. My main thing was "if we are renaming it, let's pick a name that makes it more distinct"

Klockworked commented 2 months ago

I agree that creating random aliases for parts of the standard library should be discouraged, but the reality is that people will do it to some degree, regardless of the disapproval. I don't think it should be possible to confuse these names in the first place. Personally, the first idea that comes to mind, is renaming std.builtin to std.internal. I'm not experienced enough with the compiler source to recognize any naming issues in-compiler. However, from the perspective of a user, in my opinion, "internal" does a better job at communicating that its contents are related to fundamental language constructs, not parameters for a specific compilation target.

deanveloper commented 2 months ago

My issue with internal is that the name makes it feel like something that's unsafe and shouldn't be used, when in reality it has a lot of useful utilities in it that are fine to use. The reason I picked that to be std.zig is because the package contains functions/constructs pertaining to the zig language. I think std.lang could also be a good candidate.

Also -- I don't think the whole "making aliases for standard library" part is really the pain point here. The pain point IMO is that std.builtin doesn't effectively communicate what's inside of the package, especially when @import("builtin") and "builtin functions" both exist, and both mean completely different things.

(Another option could honestly be to just merge std.meta and std.builtin considering that they're both packages which contain metaprogramming utilities... although std.builtin does have a few things that wouldn't make sense in std.meta, like the std.builtin.panicXyz methods)

Klockworked commented 2 months ago

I do understand how internal could be interpreted that way. I'm pretty sure std.zig already exists, but I have to say, I actually really like std.lang as a potential candidate. Just throwing some extra darts at the wall, what about std.core or std.primitive?

Lking03x commented 2 months ago

We could only rename @import("builtin"); to

@import("build_info"); or @import("build_constants");

leecannon commented 2 months ago

It feels weird that the Zig language is currently tied to the standard library by std.builtin.

If status quo is maintained the Zig language specification is going to have to define the contents of the std.builtin module of the standard library, instead of having no references to the standard library in the language spec at all.

The Zig language could be made independent of the standard library by:

BratishkaErik commented 1 month ago

How about @import("me")? It's recognizable (machine/module environment, just me etc.) and very short, which should help people to use it correctly and respect different module settings in different callsites:

switch (@import("me").mode) {
    // ...
}
SeanTheGleaming commented 3 weeks ago

It feels weird that the Zig language is currently tied to the standard library by std.builtin.

If status quo is maintained the Zig language specification is going to have to define the contents of the std.builtin module of the standard library, instead of having no references to the standard library in the language spec at all.

The Zig language could be made independent of the standard library by:

* Changing `@import("builtin")` to something that makes it clear that this is per build information like `@import("build_info");`

* Removing `std.builtin` from the stdlib and instead the compiler provides it as `@import("builtin")` or even `@import("builtins")` so code still referring to `@import("builtin")` fails

Even though I now agree that fully writing out the namespace you want is a sufficient solution to the original issue, I think this reply is onto something. I like this idea, for one both because it solves my original problem in a clean and readable way, but also because it solves a much larger problem I hadn't even considered when I original wrote this issue.

The Zig language depending on the namespace of std.builtin (or really just the standard library in general) is problematic for a couple reasons. As @leecannon mentioned, it would mean defining at least part of the standard library in the language spec. Additionally, it could create confusion and ambiguity in cases where multiple modules in the same compilation have the same value for std.builtin, but different values / behaviors for @import("builtin") / target specific language features. For example, it could create a conflict on whether std.builtin.VaList == @TypeOf(@cVaStart()) between multiple modules of different targets.

To try to summarize simply, if the contents of @import("builtin") can change between modules, but the contents of std.builtin cannot, then this can create conflict and ambiguity down to the language itself. As @leecannon suggested, it might be a good idea to divorce std.builtin from std altogether. This would have the added benefit of completely removing the language's dependence on the @import("std") namespace