Skip to content

std.zig.fmtId: conditionally escape primitives/_ (breaking) #18920

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 3 commits into from
Apr 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions lib/compiler/aro_translate_c/ast.zig
Original file line number Diff line number Diff line change
Expand Up @@ -828,7 +828,7 @@ const Context = struct {
fn addIdentifier(c: *Context, bytes: []const u8) Allocator.Error!TokenIndex {
if (std.zig.primitives.isPrimitive(bytes))
return c.addTokenFmt(.identifier, "@\"{s}\"", .{bytes});
return c.addTokenFmt(.identifier, "{s}", .{std.zig.fmtId(bytes)});
return c.addTokenFmt(.identifier, "{p}", .{std.zig.fmtId(bytes)});
}

fn listToSpan(c: *Context, list: []const NodeIndex) Allocator.Error!NodeSubRange {
Expand Down Expand Up @@ -2106,7 +2106,7 @@ fn renderRecord(c: *Context, node: Node) !NodeIndex {
members[1] = 0;

for (payload.fields, 0..) |field, i| {
const name_tok = try c.addTokenFmt(.identifier, "{s}", .{std.zig.fmtId(field.name)});
const name_tok = try c.addTokenFmt(.identifier, "{p}", .{std.zig.fmtId(field.name)});
_ = try c.addToken(.colon, ":");
const type_expr = try renderNode(c, field.type);

Expand Down Expand Up @@ -2199,7 +2199,7 @@ fn renderFieldAccess(c: *Context, lhs: NodeIndex, field_name: []const u8) !NodeI
.main_token = try c.addToken(.period, "."),
.data = .{
.lhs = lhs,
.rhs = try c.addTokenFmt(.identifier, "{s}", .{std.zig.fmtId(field_name)}),
.rhs = try c.addTokenFmt(.identifier, "{p}", .{std.zig.fmtId(field_name)}),
},
});
}
Expand Down
8 changes: 4 additions & 4 deletions lib/compiler/test_runner.zig
Original file line number Diff line number Diff line change
Expand Up @@ -106,10 +106,10 @@ fn mainServer() !void {
.fail = fail,
.skip = skip,
.leak = leak,
.log_err_count = std.math.lossyCast(std.meta.FieldType(
std.zig.Server.Message.TestResults.Flags,
.log_err_count,
), log_err_count),
.log_err_count = std.math.lossyCast(
@TypeOf(@as(std.zig.Server.Message.TestResults.Flags, undefined).log_err_count),
log_err_count,
),
},
});
},
Expand Down
12 changes: 6 additions & 6 deletions lib/std/Build/Step/Options.zig
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ fn printType(self: *Options, out: anytype, comptime T: type, value: T, indent: u
try printEnum(self, out, T, info, indent);

if (name) |some| {
try out.print("pub const {}: {s} = .{s};\n", .{
try out.print("pub const {}: {} = .{p_};\n", .{
std.zig.fmtId(some),
std.zig.fmtId(@typeName(T)),
std.zig.fmtId(@tagName(value)),
Expand All @@ -246,7 +246,7 @@ fn printType(self: *Options, out: anytype, comptime T: type, value: T, indent: u
try printStruct(self, out, T, info, indent);

if (name) |some| {
try out.print("pub const {}: {s} = ", .{
try out.print("pub const {}: {} = ", .{
std.zig.fmtId(some),
std.zig.fmtId(@typeName(T)),
});
Expand Down Expand Up @@ -279,7 +279,7 @@ fn printEnum(self: *Options, out: anytype, comptime T: type, comptime val: std.b

inline for (val.fields) |field| {
try out.writeByteNTimes(' ', indent);
try out.print(" {} = {d},\n", .{ std.zig.fmtId(field.name), field.value });
try out.print(" {p} = {d},\n", .{ std.zig.fmtId(field.name), field.value });
}

if (!val.is_exhaustive) {
Expand Down Expand Up @@ -313,9 +313,9 @@ fn printStruct(self: *Options, out: anytype, comptime T: type, comptime val: std

// If the type name doesn't contains a '.' the type is from zig builtins.
if (std.mem.containsAtLeast(u8, type_name, 1, ".")) {
try out.print(" {}: {}", .{ std.zig.fmtId(field.name), std.zig.fmtId(type_name) });
try out.print(" {p_}: {}", .{ std.zig.fmtId(field.name), std.zig.fmtId(type_name) });
} else {
try out.print(" {}: {s}", .{ std.zig.fmtId(field.name), type_name });
try out.print(" {p_}: {s}", .{ std.zig.fmtId(field.name), type_name });
}

if (field.default_value != null) {
Expand Down Expand Up @@ -355,7 +355,7 @@ fn printStructValue(self: *Options, out: anytype, comptime struct_val: std.built
} else {
inline for (struct_val.fields) |field| {
try out.writeByteNTimes(' ', indent);
try out.print(" .{} = ", .{std.zig.fmtId(field.name)});
try out.print(" .{p_} = ", .{std.zig.fmtId(field.name)});

const field_name = @field(val, field.name);
switch (@typeInfo(@TypeOf(field_name))) {
Expand Down
97 changes: 86 additions & 11 deletions lib/std/zig.zig
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ pub const Tokenizer = tokenizer.Tokenizer;
pub const string_literal = @import("zig/string_literal.zig");
pub const number_literal = @import("zig/number_literal.zig");
pub const primitives = @import("zig/primitives.zig");
pub const isPrimitive = primitives.isPrimitive;
pub const Ast = @import("zig/Ast.zig");
pub const AstGen = @import("zig/AstGen.zig");
pub const Zir = @import("zig/Zir.zig");
Expand Down Expand Up @@ -728,20 +729,87 @@ const tokenizer = @import("zig/tokenizer.zig");
const assert = std.debug.assert;
const Allocator = std.mem.Allocator;

/// Return a Formatter for a Zig identifier
/// Return a Formatter for a Zig identifier, escaping it with `@""` syntax if needed.
///
/// - An empty `{}` format specifier escapes invalid identifiers, identifiers that shadow primitives
/// and the reserved `_` identifier.
/// - Add `p` to the specifier to render identifiers that shadow primitives unescaped.
/// - Add `_` to the specifier to render the reserved `_` identifier unescaped.
/// - `p` and `_` can be combined, e.g. `{p_}`.
///
pub fn fmtId(bytes: []const u8) std.fmt.Formatter(formatId) {
return .{ .data = bytes };
}

/// Print the string as a Zig identifier escaping it with @"" syntax if needed.
test fmtId {
const expectFmt = std.testing.expectFmt;
try expectFmt("@\"while\"", "{}", .{fmtId("while")});
try expectFmt("@\"while\"", "{p}", .{fmtId("while")});
try expectFmt("@\"while\"", "{_}", .{fmtId("while")});
try expectFmt("@\"while\"", "{p_}", .{fmtId("while")});
try expectFmt("@\"while\"", "{_p}", .{fmtId("while")});

try expectFmt("hello", "{}", .{fmtId("hello")});
try expectFmt("hello", "{p}", .{fmtId("hello")});
try expectFmt("hello", "{_}", .{fmtId("hello")});
try expectFmt("hello", "{p_}", .{fmtId("hello")});
try expectFmt("hello", "{_p}", .{fmtId("hello")});

try expectFmt("@\"type\"", "{}", .{fmtId("type")});
try expectFmt("type", "{p}", .{fmtId("type")});
try expectFmt("@\"type\"", "{_}", .{fmtId("type")});
try expectFmt("type", "{p_}", .{fmtId("type")});
try expectFmt("type", "{_p}", .{fmtId("type")});

try expectFmt("@\"_\"", "{}", .{fmtId("_")});
try expectFmt("@\"_\"", "{p}", .{fmtId("_")});
try expectFmt("_", "{_}", .{fmtId("_")});
try expectFmt("_", "{p_}", .{fmtId("_")});
try expectFmt("_", "{_p}", .{fmtId("_")});

try expectFmt("@\"i123\"", "{}", .{fmtId("i123")});
try expectFmt("i123", "{p}", .{fmtId("i123")});
try expectFmt("@\"4four\"", "{}", .{fmtId("4four")});
try expectFmt("_underscore", "{}", .{fmtId("_underscore")});
try expectFmt("@\"11\\\"23\"", "{}", .{fmtId("11\"23")});
try expectFmt("@\"11\\x0f23\"", "{}", .{fmtId("11\x0F23")});

// These are technically not currently legal in Zig.
try expectFmt("@\"\"", "{}", .{fmtId("")});
try expectFmt("@\"\\x00\"", "{}", .{fmtId("\x00")});
}

/// Print the string as a Zig identifier, escaping it with `@""` syntax if needed.
fn formatId(
bytes: []const u8,
comptime unused_format_string: []const u8,
comptime fmt: []const u8,
options: std.fmt.FormatOptions,
writer: anytype,
) !void {
_ = unused_format_string;
if (isValidId(bytes)) {
const allow_primitive, const allow_underscore = comptime parse_fmt: {
var allow_primitive = false;
var allow_underscore = false;
for (fmt) |char| {
switch (char) {
'p' => if (!allow_primitive) {
allow_primitive = true;
continue;
},
'_' => if (!allow_underscore) {
allow_underscore = true;
continue;
},
else => {},
}
@compileError("expected {}, {p}, {_}, {p_} or {_p}, found {" ++ fmt ++ "}");
}
break :parse_fmt .{ allow_primitive, allow_underscore };
};

if (isValidId(bytes) and
(allow_primitive or !std.zig.isPrimitive(bytes)) and
(allow_underscore or !isUnderscore(bytes)))
{
return writer.writeAll(bytes);
}
try writer.writeAll("@\"");
Expand All @@ -757,12 +825,8 @@ pub fn fmtEscapes(bytes: []const u8) std.fmt.Formatter(stringEscape) {
return .{ .data = bytes };
}

test "escape invalid identifiers" {
test fmtEscapes {
const expectFmt = std.testing.expectFmt;
try expectFmt("@\"while\"", "{}", .{fmtId("while")});
try expectFmt("hello", "{}", .{fmtId("hello")});
try expectFmt("@\"11\\\"23\"", "{}", .{fmtId("11\"23")});
try expectFmt("@\"11\\x0f23\"", "{}", .{fmtId("11\x0F23")});
try expectFmt("\\x0f", "{}", .{fmtEscapes("\x0f")});
try expectFmt(
\\" \\ hi \x07 \x11 " derp \'"
Expand Down Expand Up @@ -816,7 +880,6 @@ pub fn stringEscape(

pub fn isValidId(bytes: []const u8) bool {
if (bytes.len == 0) return false;
if (std.mem.eql(u8, bytes, "_")) return false;
for (bytes, 0..) |c, i| {
switch (c) {
'_', 'a'...'z', 'A'...'Z' => {},
Expand All @@ -836,6 +899,18 @@ test isValidId {
try std.testing.expect(isValidId("i386"));
}

pub fn isUnderscore(bytes: []const u8) bool {
return bytes.len == 1 and bytes[0] == '_';
}

test isUnderscore {
try std.testing.expect(isUnderscore("_"));
try std.testing.expect(!isUnderscore("__"));
try std.testing.expect(!isUnderscore("_foo"));
try std.testing.expect(isUnderscore("\x5f"));
try std.testing.expect(!isUnderscore("\\x5f"));
}

pub fn readSourceFileToEndAlloc(
allocator: Allocator,
input: std.fs.File,
Expand Down
4 changes: 2 additions & 2 deletions lib/std/zig/render.zig
Original file line number Diff line number Diff line change
Expand Up @@ -2847,8 +2847,8 @@ fn renderIdentifier(r: *Render, token_index: Ast.TokenIndex, space: Space, quote
return renderQuotedIdentifier(r, token_index, space, false);
}

// Special case for _ which would incorrectly be rejected by isValidId below.
if (contents.len == 1 and contents[0] == '_') switch (quote) {
// Special case for _.
if (std.zig.isUnderscore(contents)) switch (quote) {
.eagerly_unquote => return renderQuotedIdentifier(r, token_index, space, true),
.eagerly_unquote_except_underscore,
.preserve_when_shadowing,
Expand Down
29 changes: 14 additions & 15 deletions src/Builtin.zig
Original file line number Diff line number Diff line change
Expand Up @@ -35,17 +35,17 @@ pub fn append(opts: @This(), buffer: *std.ArrayList(u8)) Allocator.Error!void {
\\/// feature detection (i.e. with `@hasDecl` or `@hasField`) over version checks.
\\pub const zig_version = std.SemanticVersion.parse(zig_version_string) catch unreachable;
\\pub const zig_version_string = "{s}";
\\pub const zig_backend = std.builtin.CompilerBackend.{};
\\pub const zig_backend = std.builtin.CompilerBackend.{p_};
\\
\\pub const output_mode = std.builtin.OutputMode.{};
\\pub const link_mode = std.builtin.LinkMode.{};
\\pub const output_mode = std.builtin.OutputMode.{p_};
\\pub const link_mode = std.builtin.LinkMode.{p_};
\\pub const is_test = {};
\\pub const single_threaded = {};
\\pub const abi = std.Target.Abi.{};
\\pub const abi = std.Target.Abi.{p_};
\\pub const cpu: std.Target.Cpu = .{{
\\ .arch = .{},
\\ .model = &std.Target.{}.cpu.{},
\\ .features = std.Target.{}.featureSet(&[_]std.Target.{}.Feature{{
\\ .arch = .{p_},
\\ .model = &std.Target.{p_}.cpu.{p_},
\\ .features = std.Target.{p_}.featureSet(&[_]std.Target.{p_}.Feature{{
\\
, .{
build_options.version,
Expand All @@ -66,14 +66,14 @@ pub fn append(opts: @This(), buffer: *std.ArrayList(u8)) Allocator.Error!void {
const index = @as(std.Target.Cpu.Feature.Set.Index, @intCast(index_usize));
const is_enabled = target.cpu.features.isEnabled(index);
if (is_enabled) {
try buffer.writer().print(" .{},\n", .{std.zig.fmtId(feature.name)});
try buffer.writer().print(" .{p_},\n", .{std.zig.fmtId(feature.name)});
}
}
try buffer.writer().print(
\\ }}),
\\}};
\\pub const os = std.Target.Os{{
\\ .tag = .{},
\\ .tag = .{p_},
\\ .version_range = .{{
,
.{std.zig.fmtId(@tagName(target.os.tag))},
Expand Down Expand Up @@ -180,8 +180,8 @@ pub fn append(opts: @This(), buffer: *std.ArrayList(u8)) Allocator.Error!void {
const link_libc = opts.link_libc;

try buffer.writer().print(
\\pub const object_format = std.Target.ObjectFormat.{};
\\pub const mode = std.builtin.OptimizeMode.{};
\\pub const object_format = std.Target.ObjectFormat.{p_};
\\pub const mode = std.builtin.OptimizeMode.{p_};
\\pub const link_libc = {};
\\pub const link_libcpp = {};
\\pub const have_error_return_tracing = {};
Expand All @@ -190,7 +190,7 @@ pub fn append(opts: @This(), buffer: *std.ArrayList(u8)) Allocator.Error!void {
\\pub const position_independent_code = {};
\\pub const position_independent_executable = {};
\\pub const strip_debug_info = {};
\\pub const code_model = std.builtin.CodeModel.{};
\\pub const code_model = std.builtin.CodeModel.{p_};
\\pub const omit_frame_pointer = {};
\\
, .{
Expand All @@ -209,11 +209,10 @@ pub fn append(opts: @This(), buffer: *std.ArrayList(u8)) Allocator.Error!void {
});

if (target.os.tag == .wasi) {
const wasi_exec_model_fmt = std.zig.fmtId(@tagName(opts.wasi_exec_model));
try buffer.writer().print(
\\pub const wasi_exec_model = std.builtin.WasiExecModel.{};
\\pub const wasi_exec_model = std.builtin.WasiExecModel.{p_};
\\
, .{wasi_exec_model_fmt});
, .{std.zig.fmtId(@tagName(opts.wasi_exec_model))});
}

if (opts.is_test) {
Expand Down
2 changes: 1 addition & 1 deletion src/InternPool.zig
Original file line number Diff line number Diff line change
Expand Up @@ -488,7 +488,7 @@ pub const NullTerminatedString = enum(u32) {
if (comptime std.mem.eql(u8, specifier, "")) {
try writer.writeAll(s);
} else if (comptime std.mem.eql(u8, specifier, "i")) {
try writer.print("{}", .{std.zig.fmtId(s)});
try writer.print("{p}", .{std.zig.fmtId(s)});
} else @compileError("invalid format string '" ++ specifier ++ "' for '" ++ @typeName(NullTerminatedString) ++ "'");
}

Expand Down
2 changes: 1 addition & 1 deletion src/main.zig
Original file line number Diff line number Diff line change
Expand Up @@ -6943,7 +6943,7 @@ fn cmdFetch(
std.zig.fmtEscapes(&hex_digest),
});

const new_node_text = try std.fmt.allocPrint(arena, ".{} = {s},\n", .{
const new_node_text = try std.fmt.allocPrint(arena, ".{p_} = {s},\n", .{
std.zig.fmtId(name), new_node_init,
});

Expand Down
Loading