Skip to content
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

Improve multiple error messages #20618

Closed
wants to merge 6 commits into from
Closed
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
2 changes: 2 additions & 0 deletions lib/std/Progress.zig
Original file line number Diff line number Diff line change
Expand Up @@ -543,6 +543,8 @@ fn windowsApiUpdateThreadRun() void {
///
/// The lock is recursive; the same thread may hold the lock multiple times.
pub fn lockStdErr() void {
if (@inComptime())
@compileError("I/O cannot be used from comptime. Consider using @compileError() or @compileLog().");
stderr_mutex.lock();
clearWrittenWithEscapeCodes() catch {};
}
Expand Down
2 changes: 1 addition & 1 deletion lib/std/builtin.zig
Original file line number Diff line number Diff line change
Expand Up @@ -904,7 +904,7 @@ pub const panic_messages = struct {
pub const divide_by_zero = "division by zero";
pub const exact_division_remainder = "exact division produced remainder";
pub const inactive_union_field = "access of inactive union field";
pub const integer_part_out_of_bounds = "integer part of floating point value out of bounds";
pub const integer_part_out_of_bounds = "integer part of floating point value out of bounds or not finite";
pub const corrupt_switch = "switch on corrupt value";
pub const shift_rhs_too_big = "shift amount is greater than the type size";
pub const invalid_enum_value = "invalid enum value";
Expand Down
166 changes: 134 additions & 32 deletions lib/std/zig/AstGen.zig
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
//! Ingests an AST and produces ZIR code.
const AstGen = @This();

const Parse = @import("Parse.zig");

const std = @import("std");
const Ast = std.zig.Ast;
const mem = std.mem;
Expand Down Expand Up @@ -2216,8 +2218,8 @@ fn breakExpr(parent_gz: *GenZir, parent_scope: *Scope, node: Ast.Node.Index) Inn
}
}
if (break_label != 0) {
const label_name = try astgen.identifierTokenString(break_label);
return astgen.failTok(break_label, "label not found: '{s}'", .{label_name});
const label_name = astgen.fmtIdentifier(break_label);
return astgen.failTok(break_label, "label not found: {}", .{label_name});
} else {
return astgen.failNode(node, "break expression outside loop", .{});
}
Expand Down Expand Up @@ -2289,8 +2291,8 @@ fn continueExpr(parent_gz: *GenZir, parent_scope: *Scope, node: Ast.Node.Index)
}
}
if (break_label != 0) {
const label_name = try astgen.identifierTokenString(break_label);
return astgen.failTok(break_label, "label not found: '{s}'", .{label_name});
const label_name = astgen.fmtIdentifier(break_label);
return astgen.failTok(break_label, "label not found: {}", .{label_name});
} else {
return astgen.failNode(node, "continue expression outside loop", .{});
}
Expand Down Expand Up @@ -2400,10 +2402,8 @@ fn checkLabelRedefinition(astgen: *AstGen, parent_scope: *Scope, label: Ast.Toke
const gen_zir = scope.cast(GenZir).?;
if (gen_zir.label) |prev_label| {
if (try astgen.tokenIdentEql(label, prev_label.token)) {
const label_name = try astgen.identifierTokenString(label);
return astgen.failTokNotes(label, "redefinition of label '{s}'", .{
label_name,
}, &[_]u32{
const label_name = astgen.fmtIdentifier(label);
return astgen.failTokNotes(label, "redefinition of label {}", .{label_name}, &[_]u32{
try astgen.errNoteTok(
prev_label.token,
"previous definition here",
Expand Down Expand Up @@ -4740,8 +4740,8 @@ fn testDecl(
.top => break,
};
if (found_already == null) {
const ident_name = try astgen.identifierTokenString(test_name_token);
return astgen.failTok(test_name_token, "use of undeclared identifier '{s}'", .{ident_name});
const ident_name = astgen.fmtIdentifier(test_name_token);
return astgen.failTok(test_name_token, "use of undeclared identifier {}", .{ident_name});
}

break :blk .{ .decltest = name_str_index };
Expand Down Expand Up @@ -4935,10 +4935,17 @@ fn structDeclInner(
const bodies_start = astgen.scratch.items.len;

const node_tags = tree.nodes.items(.tag);
const is_tuple = for (container_decl.ast.members) |member_node| {
var has_any_fields = false;
var first_field_name_token: ?Ast.TokenIndex = null;
for (container_decl.ast.members) |member_node| {
const container_field = tree.fullContainerField(member_node) orelse continue;
if (container_field.ast.tuple_like) break true;
} else false;
has_any_fields = true;
if (!container_field.ast.tuple_like) {
first_field_name_token = container_field.ast.main_token;
break;
}
}
const is_tuple = has_any_fields and first_field_name_token == null;

if (is_tuple) switch (layout) {
.auto => {},
Expand Down Expand Up @@ -5003,10 +5010,15 @@ fn structDeclInner(
fields_hasher.update(tree.getNodeSource(member_node));

if (!is_tuple) {
if (member.ast.tuple_like) {
return astgen.failTokNotes(member.ast.main_token, "struct field needs a name and a type", .{}, &.{
try astgen.errNoteTok(first_field_name_token.?, "to make this a tuple type, remove all field names", .{}),
});
}

const field_name = try astgen.identAsString(member.ast.main_token);

member.convertToNonTupleLike(astgen.tree.nodes);
assert(!member.ast.tuple_like);

wip_members.appendToField(@intFromEnum(field_name));

Expand All @@ -5019,8 +5031,6 @@ fn structDeclInner(
gop.value_ptr.* = .{};
try gop.value_ptr.append(sfba_allocator, member.ast.main_token);
}
} else if (!member.ast.tuple_like) {
return astgen.failTok(member.ast.main_token, "tuple field has a name", .{});
}

const doc_comment_index = try astgen.docCommentAsString(member.firstToken());
Expand Down Expand Up @@ -8290,8 +8300,8 @@ fn localVarRef(

// Can't close over a runtime variable
if (num_namespaces_out != 0 and !local_ptr.maybe_comptime and !gz.is_typeof) {
const ident_name = try astgen.identifierTokenString(ident_token);
return astgen.failNodeNotes(ident, "mutable '{s}' not accessible from here", .{ident_name}, &.{
const ident_name = astgen.fmtIdentifier(ident_token);
return astgen.failNodeNotes(ident, "mutable {} not accessible from here", .{ident_name}, &.{
try astgen.errNoteTok(local_ptr.token_src, "declared mutable here", .{}),
try astgen.errNoteNode(capturing_namespace.node, "crosses namespace boundary here", .{}),
});
Expand Down Expand Up @@ -8345,10 +8355,8 @@ fn localVarRef(
},
.top => break,
};
if (found_already == null) {
const ident_name = try astgen.identifierTokenString(ident_token);
return astgen.failNode(ident, "use of undeclared identifier '{s}'", .{ident_name});
}
if (found_already == null)
return astgen.failNode(ident, "use of undeclared identifier {}", .{astgen.fmtIdentifier(ident_token)});

// Decl references happen by name rather than ZIR index so that when unrelated
// decls are modified, ZIR code containing references to them can be unmodified.
Expand Down Expand Up @@ -9228,8 +9236,8 @@ fn builtinCall(
.top => break,
};
if (found_already == null) {
const ident_name = try astgen.identifierTokenString(ident_token);
return astgen.failNode(params[0], "use of undeclared identifier '{s}'", .{ident_name});
const ident_name = astgen.fmtIdentifier(ident_token);
return astgen.failNode(params[0], "use of undeclared identifier {}", .{ident_name});
}
},
.field_access => {
Expand Down Expand Up @@ -11205,6 +11213,56 @@ fn rvalueInner(
}
}

fn fmtIdentifier(astgen: *AstGen, ident_token: Ast.TokenIndex) std.fmt.Formatter(formatIdentifier) {
return .{ .data = .{
.self = astgen,
.token_idx = ident_token,
} };
}

const IdentifierFmt = struct {
self: *AstGen,
token_idx: Ast.TokenIndex,
};

fn formatIdentifier(
ctx: IdentifierFmt,
comptime unused_format_string: []const u8,
_: std.fmt.FormatOptions,
writer: anytype,
) !void {
comptime assert(unused_format_string.len == 0);
const astgen = ctx.self;

const token_slice = astgen.tree.tokenSlice(ctx.token_idx);
var buf: ArrayListUnmanaged(u8) = .{};
defer buf.deinit(astgen.gpa);

const complete_slice = if (token_slice[0] == '@') blk: {
try astgen.parseStrLit(ctx.token_idx, &buf, token_slice, 1);
break :blk buf.items;
} else token_slice;

const first = complete_slice[0];
const needs_escaping = if (!std.ascii.isAlphabetic(first) and first != '_')
true
else for (complete_slice[1..]) |c| {
if (!std.ascii.isAlphanumeric(c) and c != '_') {
break true;
}
} else false;

if (needs_escaping) {
try writer.writeAll("@\"");
try std.zig.stringEscape(complete_slice, "", .{}, writer);
try writer.writeByte('"');
} else {
try writer.writeByte('\'');
try writer.writeAll(complete_slice);
try writer.writeByte('\'');
}
}

/// Given an identifier token, obtain the string for it.
/// If the token uses @"" syntax, parses as a string, reports errors if applicable,
/// and allocates the result within `astgen.arena`.
Expand Down Expand Up @@ -13814,21 +13872,65 @@ fn lowerAstErrors(astgen: *AstGen) !void {

const gpa = astgen.gpa;
const parse_err = tree.errors[0];

var msg: std.ArrayListUnmanaged(u8) = .{};
defer msg.deinit(gpa);
const err_tok = parse_err.token + @intFromBool(parse_err.token_is_prev);

const token_starts = tree.tokens.items(.start);
const token_tags = tree.tokens.items(.tag);

if (try Parse.findUnmatchedParen(astgen.gpa, token_tags)) |tok| {
const text: []const u8 = switch (token_tags[tok]) {
.l_paren => "unclosed parenthesis",
.l_brace => "unclosed curly brace",
.l_bracket => "unclosed bracket",
.r_paren => "unmatched parenthesis",
.r_brace => "unmatched curly brace",
.r_bracket => "unmatched bracket",
else => unreachable,
};
try astgen.appendErrorTok(tok, "{s}", .{text});
// Unmatched parentheses are often an underlying cause of
// otherwise more obscure errors, so we only report the parse
// error if it probably wasn't caused by this.
switch (parse_err.tag) {
.asterisk_after_ptr_deref,
.chained_comparison_operators,
.expected_inlinable,
.expected_labelable,
.expected_prefix_expr,
.expected_return_type,
.extern_fn_body,
.extra_addrspace_qualifier,
.extra_align_qualifier,
.extra_allowzero_qualifier,
.extra_const_qualifier,
.extra_volatile_qualifier,
.ptr_mod_on_array_child_type,
.invalid_bit_range,
.same_line_doc_comment,
.test_doc_comment,
.comptime_doc_comment,
.varargs_nonfinal,
.expected_continue_expr,
.mismatched_binary_op_whitespace,
.invalid_ampersand_ampersand,
.extra_for_capture,
.for_input_not_captured,
=> {},
.expected_token => if (token_tags[err_tok] != .invalid) return,
else => return,
}
}

var msg: std.ArrayListUnmanaged(u8) = .{};
defer msg.deinit(gpa);

var notes: std.ArrayListUnmanaged(u32) = .{};
defer notes.deinit(gpa);

const tok = parse_err.token + @intFromBool(parse_err.token_is_prev);
if (token_tags[tok] == .invalid) {
const bad_off: u32 = @intCast(tree.tokenSlice(tok).len);
const byte_abs = token_starts[tok] + bad_off;
try notes.append(gpa, try astgen.errNoteTokOff(tok, bad_off, "invalid byte: '{'}'", .{
if (token_tags[err_tok] == .invalid) {
const bad_off: u32 = @intCast(tree.tokenSlice(err_tok).len);
const byte_abs = token_starts[err_tok] + bad_off;
try notes.append(gpa, try astgen.errNoteTokOff(err_tok, bad_off, "invalid byte: '{'}'", .{
std.zig.fmtEscapes(tree.source[byte_abs..][0..1]),
}));
}
Expand Down
31 changes: 31 additions & 0 deletions lib/std/zig/Parse.zig
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,37 @@ pub fn parseZon(p: *Parse) !void {
};
}

pub fn findUnmatchedParen(gpa: Allocator, token_tags: []const Token.Tag) !?TokenIndex {
var stack = std.ArrayList(struct {
tag: Token.Tag,
idx: TokenIndex,
}).init(gpa);
defer stack.deinit();

for (token_tags, 0..) |t, i| {
switch (t) {
.l_paren, .l_brace, .l_bracket => try stack.append(.{ .tag = t, .idx = @intCast(i) }),
.r_paren, .r_brace, .r_bracket => {
if (stack.items.len == 0 or t != closingParen(stack.pop().tag))
return @intCast(i);
},
else => {},
}
}
if (stack.items.len > 0)
return stack.pop().idx;
return null;
}

fn closingParen(a: Token.Tag) Token.Tag {
return switch (a) {
tau-dev marked this conversation as resolved.
Show resolved Hide resolved
.l_paren => .r_paren,
.l_brace => .r_brace,
.l_bracket => .r_bracket,
else => unreachable,
};
}

/// ContainerMembers <- ContainerDeclaration* (ContainerField COMMA)* (ContainerField / ContainerDeclaration*)
///
/// ContainerDeclaration <- TestDecl / ComptimeDecl / doc_comment? KEYWORD_pub? Decl
Expand Down
14 changes: 9 additions & 5 deletions src/Sema.zig
Original file line number Diff line number Diff line change
Expand Up @@ -8666,16 +8666,20 @@ fn zirErrorUnionType(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileEr
const extra = sema.code.extraData(Zir.Inst.Bin, inst_data.payload_index).data;
const lhs_src = block.src(.{ .node_offset_bin_lhs = inst_data.src_node });
const rhs_src = block.src(.{ .node_offset_bin_rhs = inst_data.src_node });
const error_set = try sema.resolveType(block, lhs_src, extra.lhs);
// Do not resolve as type yet, so that we can give a more useful message
// when someone writes `error.XYZ` instead of `error{XYZ}` as the LHS.
const error_set = try sema.resolveConstDefinedValue(block, lhs_src, try sema.resolveInst(extra.lhs), .{
.needed_comptime_reason = "types must be comptime-known",
});
const payload = try sema.resolveType(block, rhs_src, extra.rhs);

if (error_set.zigTypeTag(mod) != .ErrorSet) {
if (error_set.typeOf(mod).ip_index != .type_type or error_set.toType().zigTypeTag(mod) != .ErrorSet) {
return sema.fail(block, lhs_src, "expected error set type, found '{}'", .{
error_set.fmt(pt),
error_set.fmtValue(pt),
});
}
try sema.validateErrorUnionPayloadType(block, payload, rhs_src);
const err_union_ty = try pt.errorUnionType(error_set, payload);
const err_union_ty = try pt.errorUnionType(error_set.toType(), payload);
return Air.internedToRef(err_union_ty.toIntern());
}

Expand Down Expand Up @@ -27071,8 +27075,8 @@ fn explainWhyTypeIsNotPacked(
.ErrorSet,
.AnyFrame,
.Optional,
.Array,
=> try sema.errNote(src_loc, msg, "type has no guaranteed in-memory representation", .{}),
.Array => try sema.errNote(src_loc, msg, "packed fields are ordered according to machine endianness, array elements are not", .{}),
.Pointer => if (ty.isSlice(mod)) {
try sema.errNote(src_loc, msg, "slices have no guaranteed in-memory representation", .{});
} else {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
comptime {
const z = error.Foo!i32;
const x: z = undefined;
_ = x;
}
comptime {
const z = i32!i32;
const x: z = undefined;
Expand All @@ -8,4 +13,5 @@ comptime {
// backend=stage2
// target=native
//
// :2:15: error: expected error set type, found 'i32'
// :2:15: error: expected error set type, found 'error.Foo'
// :7:15: error: expected error set type, found 'i32'
2 changes: 1 addition & 1 deletion test/cases/compile_errors/invalid_unicode_escape.zig
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
export fn entry() void {
const a = '\u{12z34}';
const a = '\u{12z34';
}

// error
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ export fn entry14() void {
// :3:12: error: packed structs cannot contain fields of type 'anyerror'
// :3:12: note: type has no guaranteed in-memory representation
// :8:12: error: packed structs cannot contain fields of type '[2]u24'
// :8:12: note: type has no guaranteed in-memory representation
// :8:12: note: packed fields are ordered according to machine endianness, array elements are not
// :13:20: error: packed structs cannot contain fields of type 'anyerror!u32'
// :13:20: note: type has no guaranteed in-memory representation
// :18:12: error: packed structs cannot contain fields of type 'tmp.S'
Expand Down
3 changes: 2 additions & 1 deletion test/cases/compile_errors/tuple_declarations.zig
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ const T = struct {
//
// :2:5: error: enum field missing name
// :5:5: error: union field missing name
// :8:5: error: tuple field has a name
// :9:5: error: struct field needs a name and a type
// :8:5: note: to make this a tuple type, remove all field names
// :15:5: error: tuple declarations cannot contain declarations
// :12:5: note: tuple field here
Loading