Hejsil / zig-clap

Command line argument parsing library
MIT License
939 stars 67 forks source link

Remove default allocator assignment in clap #111

Closed RossComputerGuy closed 10 months ago

RossComputerGuy commented 10 months ago

I am working on a project which uses clap in UEFI and having the default allocator in the structs is causing this error despite me overriding the allocator.

/home/ross/zig/lib/zig/std/os.zig:283:25: error: root struct of file 'os.uefi' has no member named 'getErrno'
pub const errno = system.getErrno;
                  ~~~~~~^~~~~~~~~
/home/ross/zig/lib/zig/std/os/uefi.zig:1:1: note: struct declared here
const std = @import("../std.zig");
^~~~~
referenced by:
    munmap: /home/ross/zig/lib/zig/std/os.zig:4539:13
    free: /home/ross/zig/lib/zig/std/heap/PageAllocator.zig:108:11
    vtable: /home/ross/zig/lib/zig/std/heap/PageAllocator.zig:13:13
    page_allocator: /home/ross/zig/lib/zig/std/heap.zig:241:33
    ParseOptions: /home/ross/.cache/zig/p/122066d86920926a4a674adaaa10f158dc4961c2a47e588e605765455d74ca72c2ad/clap.zig:649:36
    ParseOptions: /home/ross/.cache/zig/p/122066d86920926a4a674adaaa10f158dc4961c2a47e588e605765455d74ca72c2ad/clap.zig:643:26
    parseEx: /home/ross/.cache/zig/p/122066d86920926a4a674adaaa10f158dc4961c2a47e588e605765455d74ca72c2ad/clap.zig:728:10
    parse__anon_2995: /home/ross/.cache/zig/p/122066d86920926a4a674adaaa10f158dc4961c2a47e588e605765455d74ca72c2ad/clap.zig:666:24
    main: src/ziggybox.zig:20:25
    EfiMain: /home/ross/zig/lib/zig/std/start.zig:231:37

Best option is to make the allocator nullable and doing an orelse on it.

efjimm commented 10 months ago

I don't think this field should even have a default value, the user should be forced to use an allocator. This matches how the entire standard library works and prevents any performance hiccups from unknowingly using the page allocator.

Hejsil commented 10 months ago

I don't think this field should even have a default value, the user should be forced to use an allocator. This matches how the entire standard library works and prevents any performance hiccups from unknowingly using the page allocator.

Yea, I agree. And since the defaulting breaks certain platforms I see no reason to keep this allocator default.