Skip to content

std.os reorganization; new usingnamespace semantics #9618

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

Merged
merged 28 commits into from
Sep 2, 2021
Merged

Conversation

andrewrk
Copy link
Member

@andrewrk andrewrk commented Aug 24, 2021

Originally, the main purpose of this branch was to explore avoiding the
usingnamespace feature of the zig language, specifically with regards
to std.os and related functionality.

Ultimately the decision resulted in the accepted proposal #9629. This branch now implements that proposal via a new compile error in AstGen.zig: "use of undeclared identifier". This requires sweeping changes to the standard library, so I took the opportunity to do a reorganization.

This is progress towards closing #6600 since it clarifies where the
actual "owner" of each declaration is, and reduces the number of
different ways to import the same declarations.

One of the main organizational strategies used here is to do namespacing
with real namespaces (e.g. structs) rather than by having declarations
share a common prefix (the C strategy). It's no coincidence that
usingnamespace has similar semantics to #include and becomes much
less necessary when using proper namespaces.

@andrewrk andrewrk added breaking Implementing this issue could cause existing code to no longer compile or have different behavior. standard library This issue involves writing Zig code for the standard library. labels Aug 24, 2021
@michal-z
Copy link
Contributor

michal-z commented Aug 24, 2021

In my code I use 'usingnamespace' for COM interfaces to minimize the amount of code duplication (see example below). Each interface defines 'Methods' function which is then used in each child interface.

How could I accomplish something similar without 'usingnamespace'?

Also, I use this keyword to export symbols from several different files into one common namespace, for example:

// win32.zig
pub usingnamespace @import("std").os.windows;
pub usingnamespace @import("misc.zig");
pub usingnamespace @import("d3d12.zig");
pub usingnamespace @import("d3d12sdklayers.zig");
pub usingnamespace @import("d3dcommon.zig");
pub usingnamespace @import("dxgiformat.zig");
pub usingnamespace @import("dxgitype.zig");
pub usingnamespace @import("dxgicommon.zig");
pub usingnamespace @import("dxgi.zig");
pub usingnamespace @import("dxgi1_2.zig");
// main.zig
const w = @import("win32.zig");

var device: ?*w.ID3D12Device9 = null;

(Edit) I could put each file in its own namespace. But, is it practical? Consider some names:

w.dxgiformat.DXGI_FORMAT_UNKNOWN
w.dxgi1_2.IDXGISwapChain2
w.dxgicommon.DXGI_RATIONAL
w.d3d12.ID3D12Device9
pub const ID3D12Object = extern struct {
    const Self = @This();
    v: *const extern struct {
        unknown: IUnknown.VTable(Self),
        object: VTable(Self),
    },
    usingnamespace IUnknown.Methods(Self);
    usingnamespace Methods(Self);

    fn Methods(comptime T: type) type {
        return extern struct {
            pub inline fn GetPrivateData(self: *T, guid: *const GUID, data_size: *UINT, data: ?*c_void) HRESULT {
                return self.v.object.GetPrivateData(self, guid, data_size, data);
            }
            pub inline fn SetPrivateData(self: *T, guid: *const GUID, data_size: UINT, data: ?*const c_void) HRESULT {
                return self.v.object.SetPrivateData(self, guid, data_size, data);
            }
            pub inline fn SetPrivateDataInterface(self: *T, guid: *const GUID, data: ?*const IUnknown) HRESULT {
                return self.v.object.SetPrivateDataInterface(self, guid, data);
            }
            pub inline fn SetName(self: *T, name: LPCWSTR) HRESULT {
                return self.v.object.SetName(self, name);
            }
        };
    }

    fn VTable(comptime T: type) type {
        return extern struct {
            GetPrivateData: fn (*T, *const GUID, *UINT, ?*c_void) callconv(WINAPI) HRESULT,
            SetPrivateData: fn (*T, *const GUID, UINT, ?*const c_void) callconv(WINAPI) HRESULT,
            SetPrivateDataInterface: fn (*T, *const GUID, ?*const IUnknown) callconv(WINAPI) HRESULT,
            SetName: fn (*T, LPCWSTR) callconv(WINAPI) HRESULT,
        };
    }
};

@michal-z
Copy link
Contributor

michal-z commented Aug 24, 2021

Sorry if my comment above was confusing. I'm concerned about removing 'usingnamespace' keyword from the language.
I understand that it can be problematic when overused but it is also very powerful and useful in my opinion.

@andrewrk
Copy link
Member Author

Thanks @michal-z - this is exactly the kind of examples I was looking for. I don't intend to yank the rug out from anyone, this is just an exploration.

Once all unnecessary uses of usingnamespace are removed, the valid use cases which remain will help inform whether the feature should be adjusted and if so, in what manner.

@andrewrk
Copy link
Member Author

andrewrk commented Aug 25, 2021

So far 116 uses of usingnamespace have been brought down to 56.

I have mixed feelings about this branch, but I do think it's at least worth getting it to a mergeable state and then evaluating it.

@andrewrk
Copy link
Member Author

With the latest commit:

  • down to 38 uses of usingnamespace
  • x86_64-linux behavior and std lib tests are passing with and without -lc
  • stage2 is building from source on x86_64-linux
  • all other OS's and ISA's still need more updates

@zigazeljko
Copy link
Contributor

The valid use cases which remain will help inform whether the feature should be adjusted and if so, in what manner.

Looking around the standard library, I found two (valid) use cases: conditional exports (eg. exportWhen inside std.Atomic) and mixins.

  • Conditional exports could be implemented by adding a visible_if(...) attribute for conditionally hiding declarations.
  • Mixins could be implemented by adding a mixin(...) attribute to struct/union/enum declarations.

Example syntax:

pub visible_if(T == u8) const Writer = std.io.Writer(*Self, error{OutOfMemory}, appendWrite);

pub visible_if(T == u8) fn writer(self: *Self) Writer {
    return .{ .context = self };
}
pub fn IndexedSet(comptime I: type, comptime Ext: fn (type) type) type {
    return struct mixin (pub Ext(Self)) {
        const Self = @This();
        ...
    };
}

@marler8997
Copy link
Contributor

@zigazeljko wouldn't mixin have the same drawbacks as usingnamespace? I think the point of removing usingnamespace is not to allow implicit symbols to be added to the current namespace. Andrew correct me if I'm wrong on that.

I think the "conditional export" problem would also need a solution. visible_if or something like it might work. It adds boilerplate if you want to do this for alot of symbols in a row, would be nice if we could find a way to avoid that.

@zigazeljko
Copy link
Contributor

@marler8997 I didn't really specify what mixin does. It adds the declarations from the operand to the resulting type (like usingnamespace), but it does not add them to the current scope. In other words, they are not visible as symbols in the current scope and are only accessible using the field access syntax (Self.foo in the example above).

I chose the visible_if syntax since it does not need additional indentation. The other option would be to allow if blocks around declarations.

@ifreund
Copy link
Member

ifreund commented Aug 25, 2021

It adds the declarations from the operand to the resulting type (like usingnamespace), but it does not add them to the current scope. In other words, they are not visible as symbols in the current scope and are only accessible using the field access syntax (Self.foo in the example above).

I think those semantics are an excellent idea if we choose to keep something usingnamespace-like such as your proposed mixin keyword to support the current mixin pattern. This would allow us to report "use of undeclared identifier" errors purely in AstGen.

@tauoverpi
Copy link
Contributor

@zigazeljko would using @compileError() be clearer in the conditional export case? It's more work than visible_if and quite a bit more than usingnamespace but it communicates why the decl is unavailable which is a bit nicer when one stumbles upon the error.

With visible_if the user has to go through the source to see why it's not visible (e.g platform specific conditional exports) unless visible_if shows the condition in the compilation error in a clear way.

pub const Writer = if (T != u8)
    @compileError("The Writer interface is only defined for ArrayList(u8) " ++
        "but the given type is ArrayList(" ++ @typeName(T) ++ ")")
else
    std.io.Writer(*Self, error{OutOfMemory}, appendWrite);

pub const writer = if (T != u8)
    @compileError("The Writer interface is only defined for ArrayList(u8) " ++
        "but the given type is ArrayList(" ++ @typeName(T) ++ ")")
else
    (struct {
        pub fn writer(self: *Self) Writer {
            return .{ .context = self };
        }
    }).writer;

// #1717
// pub const writer = if (T != u8)
//     @compileError("The Writer interface is only defined for ArrayList(u8) " ++
//         "but the given type is ArrayList(" ++ @typeName(T) ++ ")")
// else
//     fn(self: *Self) Writer {
//         return .{ .context = self };
//     };

I guess this depends on #513 to work with comptime.

@michal-z
Copy link
Contributor

@zigazeljko @ifreund What if the user really want to put multiple symbols from different places into one common namespace? Would mixin keyword support this? Please consider my use case below (some code omitted for clarity).
ID3D12QueryHeap needs all the functions from parent interfaces. All those functions needs to be in one common namespace.

// IUnknown
  usingnamespace Methods(Self);
  
  pub fn Methods(comptime T: type) type {
      return extern struct {
          pub inline fn QueryInterface(self: *T, guid: *const GUID, outobj: ?*?*c_void) HRESULT {
              return self.v.unknown.QueryInterface(self, guid, outobj);
          }
          pub inline fn AddRef(self: *T) ULONG {
              return self.v.unknown.AddRef(self);
          }
          pub inline fn Release(self: *T) ULONG {
              return self.v.unknown.Release(self);
          }
      };
  }

// ID3D12Object
    usingnamespace IUnknown.Methods(Self);
    usingnamespace Methods(Self);

    fn Methods(comptime T: type) type {
        return extern struct {
            pub inline fn GetPrivateData(self: *T, guid: *const GUID, data_size: *UINT, data: ?*c_void) HRESULT {
                return self.v.object.GetPrivateData(self, guid, data_size, data);
            }
            pub inline fn SetPrivateData(self: *T, guid: *const GUID, data_size: UINT, data: ?*const c_void) HRESULT {
                return self.v.object.SetPrivateData(self, guid, data_size, data);
            }
            pub inline fn SetPrivateDataInterface(self: *T, guid: *const GUID, data: ?*const IUnknown) HRESULT {
                return self.v.object.SetPrivateDataInterface(self, guid, data);
            }
            pub inline fn SetName(self: *T, name: LPCWSTR) HRESULT {
                return self.v.object.SetName(self, name);
            }
        };
    }

// ID3D12QueryHeap
    usingnamespace IUnknown.Methods(Self);
    usingnamespace ID3D12Object.Methods(Self);
    usingnamespace ID3D12DeviceChild.Methods(Self);
    usingnamespace ID3D12Pageable.Methods(Self);
    usingnamespace Methods(Self);

    fn Methods(comptime T: type) type {
        _ = T;
        return extern struct {};
    }

@andrewrk
Copy link
Member Author

andrewrk commented Aug 25, 2021

hmmm. everywhere that mixins are used in the standard library, I find the code difficult to read. And when I try to understand why the mixin is used, I come to the conclusion that it would have been better to implement without using mixins. I'm looking specifically at std/x/os/socket.zig right now.

I don't understand enums.zig enough to critique it, but maybe that itself is my critique. It's not doing something fundamentally complicated; the code should be simple to read.

@michal-z's use case looks legitimate though.

@andrewrk
Copy link
Member Author

With the latest commit:

  • Down to 19 uses of usingnamespace in all of std.
  • x86_64-windows std lib and behavior tests are passing, without -lc
  • when adding -lc there is an interesting problem: we are doing feature detection with @hasDecl which no longer works with the strategy this branch uses to avoid usingnamespace.

@michal-z
Copy link
Contributor

michal-z commented Aug 26, 2021

@andrewrk Regarding my use case above - maybe we could alter usingnamespace to behave like this:

// Inside of the struct
const Self = @This();
usingnamespace(Self, IUnknown.Methods(Self));
usingnamespace(Self, ID3D12Object.Methods(Self));
usingnamespace(Self, ID3D12DeviceChild.Methods(Self));
usingnamespace(Self, ID3D12Pageable.Methods(Self));

Or

// Outside of the struct
usingnamespace(ID3D12QueryHeap, IUnknown.Methods(ID3D12QueryHeap));
usingnamespace(ID3D12QueryHeap, ID3D12Object.Methods(ID3D12QueryHeap));
usingnamespace(ID3D12QueryHeap, ID3D12DeviceChild.Methods(ID3D12QueryHeap));
usingnamespace(ID3D12QueryHeap, ID3D12Pageable.Methods(ID3D12QueryHeap));

The first argument is a destination namespace and the second one is a namespace that we want to put into dest.
This can be more readable and more explicit than current usingnamespace.
With the proposed definition we can't just fetch a namespace and put it into current namespace - we can only put stuff into existing, named namespaces (so I guess it would behave similar to mixin keyword proposed by @zigazeljko ?).

Does this make sense? What do you think? Not sure if this solves original problem with current definition of usingnamespace.

@marler8997
Copy link
Contributor

marler8997 commented Aug 26, 2021

@michal-z by the way, zigwin32 uses a very similar method for its COM API that utilizes usingnamespace as well (here's its version of ID3D12Object: https://github.com/marlersoft/zigwin32/blob/b2cdb54fd83caf8679b8370ead57332df40e90df/win32/graphics/direct3d12.zig#L791).

pub const ID3D12Object = extern struct {
    pub const VTable = extern struct {
        base: IUnknown.VTable,
        GetPrivateData: fn(
            self: *const ID3D12Object,
            guid: ?*const Guid,
            pDataSize: ?*u32,
            // TODO: what to do with BytesParamIndex 1?
            pData: ?*c_void,
        ) callconv(@import("std").os.windows.WINAPI) HRESULT,
        SetPrivateData: fn(
            self: *const ID3D12Object,
            guid: ?*const Guid,
            DataSize: u32,
            // TODO: what to do with BytesParamIndex 1?
            pData: ?*const c_void,
        ) callconv(@import("std").os.windows.WINAPI) HRESULT,
        SetPrivateDataInterface: fn(
            self: *const ID3D12Object,
            guid: ?*const Guid,
            pData: ?*IUnknown,
        ) callconv(@import("std").os.windows.WINAPI) HRESULT,
        SetName: fn(
            self: *const ID3D12Object,
            Name: ?[*:0]const u16,
        ) callconv(@import("std").os.windows.WINAPI) HRESULT,
    };
    vtable: *const VTable,
    pub fn MethodMixin(comptime T: type) type { return struct {
        pub usingnamespace IUnknown.MethodMixin(T);
        // NOTE: method is namespaced with interface name to avoid conflicts for now
        pub fn ID3D12Object_GetPrivateData(self: *const T, guid: ?*const Guid, pDataSize: ?*u32, pData: ?*c_void) callconv(.Inline) HRESULT {
            return @ptrCast(*const ID3D12Object.VTable, self.vtable).GetPrivateData(@ptrCast(*const ID3D12Object, self), guid, pDataSize, pData);
        }
        // NOTE: method is namespaced with interface name to avoid conflicts for now
        pub fn ID3D12Object_SetPrivateData(self: *const T, guid: ?*const Guid, DataSize: u32, pData: ?*const c_void) callconv(.Inline) HRESULT {
            return @ptrCast(*const ID3D12Object.VTable, self.vtable).SetPrivateData(@ptrCast(*const ID3D12Object, self), guid, DataSize, pData);
        }
        // NOTE: method is namespaced with interface name to avoid conflicts for now
        pub fn ID3D12Object_SetPrivateDataInterface(self: *const T, guid: ?*const Guid, pData: ?*IUnknown) callconv(.Inline) HRESULT {
            return @ptrCast(*const ID3D12Object.VTable, self.vtable).SetPrivateDataInterface(@ptrCast(*const ID3D12Object, self), guid, pData);
        }
        // NOTE: method is namespaced with interface name to avoid conflicts for now
        pub fn ID3D12Object_SetName(self: *const T, Name: ?[*:0]const u16) callconv(.Inline) HRESULT {
            return @ptrCast(*const ID3D12Object.VTable, self.vtable).SetName(@ptrCast(*const ID3D12Object, self), Name);
        }
    };}
    pub usingnamespace MethodMixin(@This());
};

@michal-z
Copy link
Contributor

michal-z commented Aug 26, 2021

@marler8997 Nice! I did d3d12, d2d1, dwrite, dxgi, WASAPI and others by hand.. Should switch to zigwin32 at some point..

@batiati
Copy link
Contributor

batiati commented Aug 26, 2021

I use usingnamespace for two reasons:

1 - usingnamespace like java's import package.*
2 - pub usingnamespace as mixins/inheritance

IMHO pub usingnamespace is a pretty neat solution for code reuse.

@andrewrk
Copy link
Member Author

andrewrk commented Aug 26, 2021

I have a concrete proposal to adjust usingnamespace at this point. I'm confident it does not break any of the use cases noted in this thread. It's been hinted at by several people already; I'm only formalizing it and explaining the concrete semantic improvements.

comment extracted to #9629

@marler8997
Copy link
Contributor

@michal-z sure I'd be happy to have another user of zigwin32 if you don't want to maintain bindings by hand as Zig evolves. I did a Zig showtime on the project if you're interested as well.

@andrewrk andrewrk changed the title std.os reorganization, avoiding usingnamespace std.os reorganization; new usingnamespace semantics Aug 28, 2021
@andrewrk
Copy link
Member Author

This branch has implications for translate-c:

/home/andy/.cache/zig/o/0f8d4e1ba937437af927113e2bcae05c/source.zig:540:33: error: use of undeclared identifier 'L'
pub const __INTMAX_C_SUFFIX__ = L;
                                ^
/home/andy/.cache/zig/o/0f8d4e1ba937437af927113e2bcae05c/source.zig:546:34: error: use of undeclared identifier 'UL'
pub const __UINTMAX_C_SUFFIX__ = UL;
                                 ^
/home/andy/.cache/zig/o/0f8d4e1ba937437af927113e2bcae05c/source.zig:634:32: error: use of undeclared identifier 'L'
pub const __INT64_C_SUFFIX__ = L;
                               ^
/home/andy/.cache/zig/o/0f8d4e1ba937437af927113e2bcae05c/source.zig:654:33: error: use of undeclared identifier 'U'
pub const __UINT32_C_SUFFIX__ = U;
                                ^
/home/andy/.cache/zig/o/0f8d4e1ba937437af927113e2bcae05c/source.zig:662:33: error: use of undeclared identifier 'UL'
pub const __UINT64_C_SUFFIX__ = UL;
                                ^
/home/andy/.cache/zig/o/0f8d4e1ba937437af927113e2bcae05c/source.zig:782:22: error: use of undeclared identifier '__attribute__'
pub const __seg_gs = __attribute__(address_space(@as(c_int, 256)));
                     ^
/home/andy/.cache/zig/o/0f8d4e1ba937437af927113e2bcae05c/source.zig:783:22: error: use of undeclared identifier '__attribute__'
pub const __seg_fs = __attribute__(address_space(@as(c_int, 257)));
                     ^
/home/andy/.cache/zig/o/0f8d4e1ba937437af927113e2bcae05c/source.zig:881:21: error: use of undeclared identifier '__attribute__'
pub const __THROW = __attribute__(__nothrow__ ++ __LEAF);
                    ^
/home/andy/.cache/zig/o/0f8d4e1ba937437af927113e2bcae05c/source.zig:882:23: error: use of undeclared identifier '__attribute__'
pub const __THROWNL = __attribute__(__nothrow__);
                      ^
/home/andy/.cache/zig/o/0f8d4e1ba937437af927113e2bcae05c/source.zig:883:65: error: use of undeclared identifier '__has_extension'
pub inline fn __glibc_clang_has_extension(ext: anytype) @TypeOf(__has_extension(ext)) {
                                                                ^
/home/andy/.cache/zig/o/0f8d4e1ba937437af927113e2bcae05c/source.zig:893:43: error: use of undeclared identifier '__builtin_object_size'
pub inline fn __bos(ptr: anytype) @TypeOf(__builtin_object_size(ptr, __USE_FORTIFY_LEVEL > @as(c_int, 1))) {
                                          ^
/home/andy/.cache/zig/o/0f8d4e1ba937437af927113e2bcae05c/source.zig:896:44: error: use of undeclared identifier '__builtin_object_size'
pub inline fn __bos0(ptr: anytype) @TypeOf(__builtin_object_size(ptr, @as(c_int, 0))) {
                                           ^
/home/andy/.cache/zig/o/0f8d4e1ba937437af927113e2bcae05c/source.zig:906:60: error: use of undeclared identifier '__USER_LABEL_PREFIX__'
pub inline fn __ASMNAME(cname: anytype) @TypeOf(__ASMNAME2(__USER_LABEL_PREFIX__, cname)) {
                                                           ^
/home/andy/.cache/zig/o/0f8d4e1ba937437af927113e2bcae05c/source.zig:909:34: error: use of undeclared identifier '__attribute__'
pub const __attribute_malloc__ = __attribute__(__malloc__);
                                 ^
/home/andy/.cache/zig/o/0f8d4e1ba937437af927113e2bcae05c/source.zig:910:32: error: use of undeclared identifier '__attribute__'
pub const __attribute_pure__ = __attribute__(__pure__);
                               ^
/home/andy/.cache/zig/o/0f8d4e1ba937437af927113e2bcae05c/source.zig:911:33: error: use of undeclared identifier '__attribute__'
pub const __attribute_const__ = __attribute__(__const__);
                                ^
/home/andy/.cache/zig/o/0f8d4e1ba937437af927113e2bcae05c/source.zig:912:32: error: use of undeclared identifier '__attribute__'
pub const __attribute_used__ = __attribute__(__used__);
                               ^
/home/andy/.cache/zig/o/0f8d4e1ba937437af927113e2bcae05c/source.zig:913:36: error: use of undeclared identifier '__attribute__'
pub const __attribute_noinline__ = __attribute__(__noinline__);
                                   ^
/home/andy/.cache/zig/o/0f8d4e1ba937437af927113e2bcae05c/source.zig:914:38: error: use of undeclared identifier '__attribute__'
pub const __attribute_deprecated__ = __attribute__(__deprecated__);
                                     ^
/home/andy/.cache/zig/o/0f8d4e1ba937437af927113e2bcae05c/source.zig:915:66: error: use of undeclared identifier '__attribute__'
pub inline fn __attribute_deprecated_msg__(msg: anytype) @TypeOf(__attribute__(__deprecated__(msg))) {
                                                                 ^
/home/andy/.cache/zig/o/0f8d4e1ba937437af927113e2bcae05c/source.zig:918:60: error: use of undeclared identifier '__attribute__'
pub inline fn __attribute_format_arg__(x: anytype) @TypeOf(__attribute__(__format_arg__(x))) {
                                                           ^
/home/andy/.cache/zig/o/0f8d4e1ba937437af927113e2bcae05c/source.zig:921:76: error: use of undeclared identifier '__attribute__'
pub inline fn __attribute_format_strfmon__(a: anytype, b: anytype) @TypeOf(__attribute__(__format__(__strfmon__, a, b))) {
                                                                           ^
/home/andy/.cache/zig/o/0f8d4e1ba937437af927113e2bcae05c/source.zig:924:50: error: use of undeclared identifier '__attribute__'
pub inline fn __nonnull(params: anytype) @TypeOf(__attribute__(__nonnull__ ++ params)) {
                                                 ^
/home/andy/.cache/zig/o/0f8d4e1ba937437af927113e2bcae05c/source.zig:927:46: error: use of undeclared identifier '__attribute__'
pub const __attribute_warn_unused_result__ = __attribute__(__warn_unused_result__);
                                             ^
/home/andy/.cache/zig/o/0f8d4e1ba937437af927113e2bcae05c/source.zig:928:29: error: use of undeclared identifier '__inline'
pub const __always_inline = __inline ++ __attribute__(__always_inline__);
                            ^
/home/andy/.cache/zig/o/0f8d4e1ba937437af927113e2bcae05c/source.zig:929:58: error: use of undeclared identifier '__attribute_artificial__'
pub const __fortify_function = __extern_always_inline ++ __attribute_artificial__;
                                                         ^
/home/andy/.cache/zig/o/0f8d4e1ba937437af927113e2bcae05c/source.zig:930:28: error: use of undeclared identifier '__restrict'
pub const __restrict_arr = __restrict;
                           ^
/home/andy/.cache/zig/o/0f8d4e1ba937437af927113e2bcae05c/source.zig:931:55: error: use of undeclared identifier '__builtin_expect'
pub inline fn __glibc_unlikely(cond: anytype) @TypeOf(__builtin_expect(cond, @as(c_int, 0))) {
                                                      ^
/home/andy/.cache/zig/o/0f8d4e1ba937437af927113e2bcae05c/source.zig:934:53: error: use of undeclared identifier '__builtin_expect'
pub inline fn __glibc_likely(cond: anytype) @TypeOf(__builtin_expect(cond, @as(c_int, 1))) {
                                                    ^
/home/andy/.cache/zig/o/0f8d4e1ba937437af927113e2bcae05c/source.zig:937:60: error: use of undeclared identifier '__has_attribute'
pub inline fn __glibc_has_attribute(attr: anytype) @TypeOf(__has_attribute(attr)) {
                                                           ^
/home/andy/.cache/zig/o/0f8d4e1ba937437af927113e2bcae05c/source.zig:964:86: error: use of undeclared identifier 'GCC'
pub inline fn __glibc_macro_warning(message: anytype) @TypeOf(__glibc_macro_warning1(GCC ++ warning ++ message)) {
                                                                                     ^
/home/andy/.cache/zig/o/0f8d4e1ba937437af927113e2bcae05c/source.zig:968:41: error: use of undeclared identifier '__attribute__'
pub const __attribute_returns_twice__ = __attribute__(__returns_twice__);
                                        ^
/home/andy/.cache/zig/o/0f8d4e1ba937437af927113e2bcae05c/source.zig:1063:47: error: use of undeclared identifier '__builtin_huge_valf'
pub inline fn __builtin_huge_valf32() @TypeOf(__builtin_huge_valf()) {
                                              ^
/home/andy/.cache/zig/o/0f8d4e1ba937437af927113e2bcae05c/source.zig:1066:42: error: use of undeclared identifier '__builtin_inff'
pub inline fn __builtin_inff32() @TypeOf(__builtin_inff()) {
                                         ^
/home/andy/.cache/zig/o/0f8d4e1ba937437af927113e2bcae05c/source.zig:1069:52: error: use of undeclared identifier '__builtin_nanf'
pub inline fn __builtin_nanf32(x: anytype) @TypeOf(__builtin_nanf(x)) {
                                                   ^

Now even for unreferenced decls such errors will be emitted.

cc @Vexu @ehaas would it be feasible to define all these things? At the very least it could put pub const foo = @compileError("unable to use this C macro in zig"); or similar for things that are untranslatable.

@ehaas
Copy link
Contributor

ehaas commented Aug 31, 2021

I just created #9664 which should take care of simple macros. Now that I think of it, we'll also need to check if names exist in the c_builtins namespace.

Function-like macros will need additional work to check if identifiers exist in the scope or as an argument or as a c_builtin. We'll also need to update how the c_builtins package gets used - it currently imports with usingnamespace but it shouldn't be terribly difficult to manually import each usage.

@ehaas
Copy link
Contributor

ehaas commented Aug 31, 2021

@andrewrk @Vexu how would you feel about inlining the contents of std/zig/c_builtins.zig into the translate-c output? Currently we do pub usingnamespace @import("std").zig.c_builtins; but with this new change something like #define FOO __builtin_popcount becomes trickier to handle because we'd have to translate it as @This().__builtin_popcount, if we includenamespace the C builtins, right? And we'd need to do the same with any of those builtins used in a function-like macro if it's used as return value. If we inline that file I think we can just use the existing isBuiltinDefined function to check if a function is translatable.

@andrewrk andrewrk merged commit 594271f into master Sep 2, 2021
@andrewrk andrewrk deleted the std-os-reorg branch September 2, 2021 08:51
kprotty added a commit to kprotty/zig that referenced this pull request Sep 2, 2021
vrischmann added a commit to vrischmann/zig-sqlite that referenced this pull request Sep 3, 2021
vrischmann added a commit to vrischmann/zig-sqlite that referenced this pull request Sep 4, 2021
vrischmann added a commit to vrischmann/zig-sqlite that referenced this pull request Sep 4, 2021
PhaseMage pushed a commit to PhaseMage/zig that referenced this pull request Oct 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking Implementing this issue could cause existing code to no longer compile or have different behavior. standard library This issue involves writing Zig code for the standard library.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants