From e666dee86b600eede513a2e170a5fe81531a4235 Mon Sep 17 00:00:00 2001 From: torque Date: Sat, 8 Apr 2023 15:13:00 -0700 Subject: [PATCH] parser: don't force pass userdata as a pointer This is an interesting change. While I think generally passing in constant userdata is not terribly useful, the previous implementation precluded it entirely. Interface types, for example, are often passed directly and stored as constants (they hold pointers to their mutable state). Since we type erase this so it can be bound to the generic interface object, non-pointer objects must be passed by reference to avoid binding the parser interface to a temporary stack copy of the object. This means we have to handle these cases slightly differently. Also, while technically being classified as pointers, slices don't really behave like pointers, which is understandable but annoying. There's a bit of asymmetry here, as CommandBuilder(*u32) and CommandBuilder (u32) both require an *u32 when binding the parser interface. This is of course because pointers do not need to be rewrapped to be type erased. The same code path could be used for both cases, but then the user would have to pass in a pointer to a pointer, which actually looks a bit silly because then it potentially means having to do &&my_var. --- demo/demo.zig | 16 +++++---- source/command.zig | 2 +- source/converters.zig | 15 ++++----- source/parser.zig | 75 ++++++++++++++++++++++++++++--------------- 4 files changed, 66 insertions(+), 42 deletions(-) diff --git a/demo/demo.zig b/demo/demo.zig index ece8355..52b746a 100644 --- a/demo/demo.zig +++ b/demo/demo.zig @@ -6,7 +6,7 @@ const CommandBuilder = noclip.CommandBuilder; const Choice = enum { first, second }; const cli = cmd: { - var cmd = CommandBuilder(u32){ + var cmd = CommandBuilder(*u32){ .description = \\The definitive noclip demonstration utility \\ @@ -71,14 +71,14 @@ const cli = cmd: { }; const subcommand = cmd: { - var cmd = CommandBuilder(void){ + var cmd = CommandBuilder([]const u8){ .description = \\Perform some sort of work \\ \\This subcommand is a mystery. It probably does something, but nobody is sure what. , }; - cmd.add_flag(.{}, .{ + cmd.simple_flag(.{ .name = "flag", .truthy = .{ .short_tag = "-f", .long_tag = "--flag" }, .falsy = .{ .long_tag = "--no-flag" }, @@ -88,15 +88,16 @@ const subcommand = cmd: { break :cmd cmd; }; -fn sub_handler(_: *void, result: subcommand.Output()) !void { +fn sub_handler(context: []const u8, result: subcommand.Output()) !void { std.debug.print("subcommand: {s}\n", .{result.argument}); + std.debug.print("context: {s}\n", .{context}); } fn cli_handler(context: *u32, result: cli.Output()) !void { - _ = context; - + std.debug.print("context: {d}\n", .{context.*}); std.debug.print("callback is working {any}\n", .{result.choice}); std.debug.print("callback is working {d}\n", .{result.default}); + context.* += 1; } pub fn main() !u8 { @@ -106,9 +107,10 @@ pub fn main() !u8 { var parser = cli.create_parser(cli_handler, allocator); var context: u32 = 2; + const sc: []const u8 = "whassup"; var subcon = subcommand.create_parser(sub_handler, allocator); - try parser.add_subcommand("verb", subcon.interface()); + try parser.add_subcommand("verb", subcon.interface(&sc)); const iface = parser.interface(&context); iface.execute() catch return 1; diff --git a/source/command.zig b/source/command.zig index a963f13..16fee87 100644 --- a/source/command.zig +++ b/source/command.zig @@ -241,7 +241,7 @@ pub fn CommandBuilder(comptime UserContext: type) type { } pub fn CallbackSignature(comptime self: @This()) type { - return *const fn (*UserContext, self.Output()) anyerror!void; + return *const fn (UserContext, self.Output()) anyerror!void; } pub fn Output(comptime self: @This()) type { diff --git a/source/converters.zig b/source/converters.zig index a77d27e..1980213 100644 --- a/source/converters.zig +++ b/source/converters.zig @@ -11,7 +11,7 @@ const ErrorWriter = std.ArrayList(u8).Writer; pub fn ConverterSignature(comptime gen: ParameterGenerics) type { return *const fn ( - context: *gen.UserContext, + context: gen.UserContext, input: gen.IntermediateType(), failure: ErrorWriter, ) ConversionError!gen.ConvertedType(); @@ -46,7 +46,7 @@ fn multi_converter(comptime gen: ParameterGenerics) ?ConverterSignature(gen) { const Intermediate = gen.IntermediateType(); return struct { - pub fn handler(context: *gen.UserContext, input: Intermediate, failure: ErrorWriter) ConversionError!std.ArrayList(gen.OutputType) { + pub fn handler(context: gen.UserContext, input: Intermediate, failure: ErrorWriter) ConversionError!std.ArrayList(gen.OutputType) { var output = std.ArrayList(gen.OutputType).initCapacity(input.allocator, input.items.len) catch return ConversionError.ConversionFailed; @@ -61,7 +61,7 @@ fn multi_converter(comptime gen: ParameterGenerics) ?ConverterSignature(gen) { fn flag_converter(comptime gen: ParameterGenerics) ConverterSignature(gen) { return struct { - pub fn handler(_: *gen.UserContext, input: []const u8, _: ErrorWriter) ConversionError!bool { + pub fn handler(_: gen.UserContext, input: []const u8, _: ErrorWriter) ConversionError!bool { // treat an empty string as falsy if (input.len == 0) return false; @@ -81,7 +81,7 @@ fn flag_converter(comptime gen: ParameterGenerics) ConverterSignature(gen) { fn string_converter(comptime gen: ParameterGenerics) ConverterSignature(gen) { return struct { - pub fn handler(_: *gen.UserContext, input: []const u8, _: ErrorWriter) ConversionError![]const u8 { + pub fn handler(_: gen.UserContext, input: []const u8, _: ErrorWriter) ConversionError![]const u8 { return input; } }.handler; @@ -91,9 +91,8 @@ fn int_converter(comptime gen: ParameterGenerics) ConverterSignature(gen) { const IntType = gen.OutputType; return struct { - pub fn handler(_: *gen.UserContext, input: []const u8, failure: ErrorWriter) ConversionError!IntType { + pub fn handler(_: gen.UserContext, input: []const u8, failure: ErrorWriter) ConversionError!IntType { return std.fmt.parseInt(IntType, input, 0) catch { - // ignore the error try failure.print("cannot interpret \"{s}\" as an integer", .{input}); return ConversionError.ConversionFailed; }; @@ -107,7 +106,7 @@ fn struct_converter(comptime gen: ParameterGenerics) ConverterSignature(gen) { const Intermediate = gen.IntermediateType(); return struct { - pub fn handler(context: *gen.UserContext, input: Intermediate, failure: ErrorWriter) ConversionError!StructType { + pub fn handler(context: gen.UserContext, input: Intermediate, failure: ErrorWriter) ConversionError!StructType { if (input.items.len != type_info.fields.len) { try failure.print( "Wrong number of fields provided. Got {d}, needed {d}", @@ -138,7 +137,7 @@ fn choice_converter(comptime gen: ParameterGenerics) ConverterSignature(gen) { const EnumType = gen.OutputType; return struct { - pub fn handler(_: *gen.UserContext, input: []const u8, failure: ErrorWriter) ConversionError!EnumType { + pub fn handler(_: gen.UserContext, input: []const u8, failure: ErrorWriter) ConversionError!EnumType { return std.meta.stringToEnum(gen.ConvertedType(), input) orelse { try failure.print("\"{s}\" is not a valid choice", .{input}); return ConversionError.ConversionFailed; diff --git a/source/parser.zig b/source/parser.zig index b354291..f793189 100644 --- a/source/parser.zig +++ b/source/parser.zig @@ -37,7 +37,9 @@ pub const ParserInterface = struct { }; fn InterfaceGen(comptime ParserType: type, comptime UserContext: type) type { - return if (@typeInfo(UserContext) == .Void) struct { + const CtxInfo = @typeInfo(UserContext); + + return if (CtxInfo == .Void) struct { pub fn interface(self: *ParserType) ParserInterface { return .{ .parser = self, @@ -50,11 +52,15 @@ fn InterfaceGen(comptime ParserType: type, comptime UserContext: type) type { }, }; } - } else struct { - pub fn interface(self: *ParserType, context: *UserContext) ParserInterface { + + fn cast_context(_: *anyopaque) void { + return void{}; + } + } else if (CtxInfo == .Pointer and CtxInfo.Pointer.size != .Slice) struct { + pub fn interface(self: *ParserType, context: UserContext) ParserInterface { return .{ .parser = self, - .context = context, + .context = @ptrCast(*anyopaque, @constCast(context)), .methods = &.{ .execute = ParserType.wrap_execute, .parse = ParserType.wrap_parse, @@ -63,6 +69,27 @@ fn InterfaceGen(comptime ParserType: type, comptime UserContext: type) type { }, }; } + + fn cast_context(ctx: *anyopaque) UserContext { + return @ptrCast(UserContext, @alignCast(std.meta.alignment(UserContext), ctx)); + } + } else struct { + pub fn interface(self: *ParserType, context: *const UserContext) ParserInterface { + return .{ + .parser = self, + .context = @ptrCast(*anyopaque, @constCast(context)), + .methods = &.{ + .execute = ParserType.wrap_execute, + .parse = ParserType.wrap_parse, + .finish = ParserType.wrap_finish, + .describe = ParserType.describe, + }, + }; + } + + fn cast_context(ctx: *anyopaque) UserContext { + return @ptrCast(*const UserContext, @alignCast(@alignOf(UserContext), ctx)).*; + } }; } @@ -93,35 +120,31 @@ pub fn Parser(comptime command: anytype, comptime callback: anytype) type { // This is a slightly annoying hack to work around the fact that there's no way to // provide a method signature conditionally. - pub usingnamespace InterfaceGen(@This(), UserContext); + const Interface = InterfaceGen(@This(), UserContext); + pub usingnamespace Interface; + + inline fn cast_interface_parser(parser: *anyopaque) *@This() { + return @ptrCast(*@This(), @alignCast(@alignOf(@This()), parser)); + } fn wrap_execute(parser: *anyopaque, ctx: *anyopaque) anyerror!void { - const self = @ptrCast(*@This(), @alignCast(@alignOf(*@This()), parser)); + const self = cast_interface_parser(parser); // this is a slightly annoying hack to work around the problem that void has // 0 alignment, which alignCast chokes on. - const context = if (@alignOf(UserContext) > 0) - @ptrCast(*UserContext, @alignCast(@alignOf(UserContext), ctx)) - else - @ptrCast(*UserContext, ctx); + const context = Interface.cast_context(ctx); return try self.execute(context); } fn wrap_parse(parser: *anyopaque, ctx: *anyopaque, name: []const u8, args: [][:0]u8, env: std.process.EnvMap) anyerror!void { - const self = @ptrCast(*@This(), @alignCast(@alignOf(@This()), parser)); - const context = if (@alignOf(UserContext) > 0) - @ptrCast(*UserContext, @alignCast(@alignOf(UserContext), ctx)) - else - @ptrCast(*UserContext, ctx); + const self = cast_interface_parser(parser); + const context = Interface.cast_context(ctx); return try self.subparse(context, name, args, env); } fn wrap_finish(parser: *anyopaque, ctx: *anyopaque) anyerror!void { - const self = @ptrCast(*@This(), @alignCast(@alignOf(@This()), parser)); - const context = if (@alignOf(UserContext) > 0) - @ptrCast(*UserContext, @alignCast(@alignOf(UserContext), ctx)) - else - @ptrCast(*UserContext, ctx); + const self = cast_interface_parser(parser); + const context = Interface.cast_context(ctx); return try self.finish(context); } @@ -129,7 +152,7 @@ pub fn Parser(comptime command: anytype, comptime callback: anytype) type { return command.description; } - pub fn subparse(self: *@This(), context: *UserContext, name: []const u8, args: [][:0]u8, env: std.process.EnvMap) anyerror!void { + pub fn subparse(self: *@This(), context: UserContext, name: []const u8, args: [][:0]u8, env: std.process.EnvMap) anyerror!void { const sliceto = try self.parse(name, args); try self.read_environment(env); try self.convert_eager(context); @@ -149,13 +172,13 @@ pub fn Parser(comptime command: anytype, comptime callback: anytype) type { } } - pub fn finish(self: *@This(), context: *UserContext) anyerror!void { + pub fn finish(self: *@This(), context: UserContext) anyerror!void { try self.convert(context); try callback(context, self.output); if (self.subcommand) |verb| try verb.finish(); } - pub fn execute(self: *@This(), context: *UserContext) anyerror!void { + pub fn execute(self: *@This(), context: UserContext) anyerror!void { const args = try std.process.argsAlloc(self.allocator); defer std.process.argsFree(self.allocator, args); var env = try std.process.getEnvMap(self.allocator); @@ -410,7 +433,7 @@ pub fn Parser(comptime command: anytype, comptime callback: anytype) type { } } - fn convert_eager(self: *@This(), context: *UserContext) NoclipError!void { + fn convert_eager(self: *@This(), context: UserContext) NoclipError!void { inline for (comptime parameters) |param| { if (comptime param.eager) { try self.convert_param(param, context); @@ -418,7 +441,7 @@ pub fn Parser(comptime command: anytype, comptime callback: anytype) type { } } - fn convert(self: *@This(), context: *UserContext) NoclipError!void { + fn convert(self: *@This(), context: UserContext) NoclipError!void { inline for (comptime parameters) |param| { if (comptime !param.eager) { try self.convert_param(param, context); @@ -426,7 +449,7 @@ pub fn Parser(comptime command: anytype, comptime callback: anytype) type { } } - fn convert_param(self: *@This(), comptime param: anytype, context: *UserContext) NoclipError!void { + fn convert_param(self: *@This(), comptime param: anytype, context: UserContext) NoclipError!void { if (@field(self.intermediate, param.name)) |intermediate| { var buffer = std.ArrayList(u8).init(self.allocator); const writer = buffer.writer();