From 41336acb0bb89dd23ef9c49b15d91a847f6796bc Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Thu, 1 Jul 2021 16:21:38 -0700 Subject: [PATCH] AstGen: detect redeclaration of function parameters Also improve redeclaration error message to include the category of variable. --- .../protocols/simple_network_protocol.zig | 8 +- src/AstGen.zig | 216 +++++++++--------- 2 files changed, 113 insertions(+), 111 deletions(-) diff --git a/lib/std/os/uefi/protocols/simple_network_protocol.zig b/lib/std/os/uefi/protocols/simple_network_protocol.zig index 8b643d2765..1cd93bc491 100644 --- a/lib/std/os/uefi/protocols/simple_network_protocol.zig +++ b/lib/std/os/uefi/protocols/simple_network_protocol.zig @@ -57,13 +57,13 @@ pub const SimpleNetworkProtocol = extern struct { } /// Modifies or resets the current station address, if supported. - pub fn stationAddress(self: *const SimpleNetworkProtocol, reset: bool, new: ?*const MacAddress) Status { - return self._station_address(self, reset, new); + pub fn stationAddress(self: *const SimpleNetworkProtocol, reset_flag: bool, new: ?*const MacAddress) Status { + return self._station_address(self, reset_flag, new); } /// Resets or collects the statistics on a network interface. - pub fn statistics(self: *const SimpleNetworkProtocol, reset_: bool, statistics_size: ?*usize, statistics_table: ?*NetworkStatistics) Status { - return self._statistics(self, reset_, statistics_size, statistics_table); + pub fn statistics(self: *const SimpleNetworkProtocol, reset_flag: bool, statistics_size: ?*usize, statistics_table: ?*NetworkStatistics) Status { + return self._statistics(self, reset_flag, statistics_size, statistics_table); } /// Converts a multicast IP address to a multicast HW MAC address. diff --git a/src/AstGen.zig b/src/AstGen.zig index 1e9abda660..c38ba755e7 100644 --- a/src/AstGen.zig +++ b/src/AstGen.zig @@ -2216,25 +2216,15 @@ fn checkUsed( .gen_zir => scope = scope.cast(GenZir).?.parent, .local_val => { const s = scope.cast(Scope.LocalVal).?; - switch (s.used) { - .used => {}, - .fn_param => return astgen.failTok(s.token_src, "unused function parameter", .{}), - .constant => return astgen.failTok(s.token_src, "unused local constant", .{}), - .variable => unreachable, - .loop_index => unreachable, - .capture => return astgen.failTok(s.token_src, "unused capture", .{}), + if (!s.used) { + return astgen.failTok(s.token_src, "unused {s}", .{@tagName(s.id_cat)}); } scope = s.parent; }, .local_ptr => { const s = scope.cast(Scope.LocalPtr).?; - switch (s.used) { - .used => {}, - .fn_param => unreachable, - .constant => return astgen.failTok(s.token_src, "unused local constant", .{}), - .variable => return astgen.failTok(s.token_src, "unused local variable", .{}), - .loop_index => return astgen.failTok(s.token_src, "unused loop index capture", .{}), - .capture => unreachable, + if (!s.used) { + return astgen.failTok(s.token_src, "unused {s}", .{@tagName(s.id_cat)}); } scope = s.parent; }, @@ -2280,63 +2270,7 @@ fn varDecl( } const ident_name = try astgen.identAsString(name_token); - // Local variables shadowing detection, including function parameters. - { - var s = scope; - while (true) switch (s.tag) { - .local_val => { - const local_val = s.cast(Scope.LocalVal).?; - if (local_val.name == ident_name) { - const name = try gpa.dupe(u8, mem.spanZ(astgen.nullTerminatedString(ident_name))); - defer gpa.free(name); - return astgen.failTokNotes(name_token, "redeclaration of '{s}'", .{ - name, - }, &[_]u32{ - try astgen.errNoteTok( - local_val.token_src, - "previously declared here", - .{}, - ), - }); - } - s = local_val.parent; - }, - .local_ptr => { - const local_ptr = s.cast(Scope.LocalPtr).?; - if (local_ptr.name == ident_name) { - const name = try gpa.dupe(u8, mem.spanZ(astgen.nullTerminatedString(ident_name))); - defer gpa.free(name); - return astgen.failTokNotes(name_token, "redeclaration of '{s}'", .{ - name, - }, &[_]u32{ - try astgen.errNoteTok( - local_ptr.token_src, - "previously declared here", - .{}, - ), - }); - } - s = local_ptr.parent; - }, - .namespace => { - const ns = s.cast(Scope.Namespace).?; - const decl_node = ns.decls.get(ident_name) orelse { - s = ns.parent; - continue; - }; - const name = try gpa.dupe(u8, mem.spanZ(astgen.nullTerminatedString(ident_name))); - defer gpa.free(name); - return astgen.failTokNotes(name_token, "local shadows declaration of '{s}'", .{ - name, - }, &[_]u32{ - try astgen.errNoteNode(decl_node, "declared here", .{}), - }); - }, - .gen_zir => s = s.cast(GenZir).?.parent, - .defer_normal, .defer_error => s = s.cast(Scope.Defer).?.parent, - .top => break, - }; - } + try astgen.detectLocalShadowing(scope, ident_name, name_token); if (var_decl.ast.init_node == 0) { return astgen.failNode(node, "variables must be initialized", .{}); @@ -2369,7 +2303,7 @@ fn varDecl( .name = ident_name, .inst = init_inst, .token_src = name_token, - .used = .constant, + .id_cat = .@"local constant", }; return &sub_scope.base; } @@ -2438,7 +2372,7 @@ fn varDecl( .name = ident_name, .inst = init_inst, .token_src = name_token, - .used = .constant, + .id_cat = .@"local constant", }; return &sub_scope.base; } @@ -2468,7 +2402,7 @@ fn varDecl( .ptr = init_scope.rl_ptr, .token_src = name_token, .maybe_comptime = true, - .used = .constant, + .id_cat = .@"local constant", }; return &sub_scope.base; }, @@ -2525,7 +2459,7 @@ fn varDecl( .ptr = var_data.alloc, .token_src = name_token, .maybe_comptime = is_comptime, - .used = .variable, + .id_cat = .@"local variable", }; return &sub_scope.base; }, @@ -3028,7 +2962,7 @@ fn fnDecl( const param_name = try astgen.identAsString(name_token); // Create an arg instruction. This is needed to emit a semantic analysis // error for shadowing decls. - // TODO emit a compile error here for shadowing locals. + try astgen.detectLocalShadowing(params_scope, param_name, name_token); const arg_inst = try fn_gz.addStrTok(.arg, param_name, name_token); const sub_scope = try astgen.arena.create(Scope.LocalVal); sub_scope.* = .{ @@ -3037,7 +2971,7 @@ fn fnDecl( .name = param_name, .inst = arg_inst, .token_src = name_token, - .used = .fn_param, + .id_cat = .@"function parameter", }; params_scope = &sub_scope.base; @@ -4701,7 +4635,7 @@ fn orelseCatchExpr( .name = err_name, .inst = try then_scope.addUnNode(unwrap_code_op, operand, node), .token_src = payload, - .used = .capture, + .id_cat = .@"capture", }; break :blk &err_val_scope.base; }; @@ -4993,7 +4927,7 @@ fn ifExpr( .name = ident_name, .inst = payload_inst, .token_src = payload_token, - .used = .capture, + .id_cat = .@"capture", }; break :s &payload_val_scope.base; } else { @@ -5015,7 +4949,7 @@ fn ifExpr( .name = ident_name, .inst = payload_inst, .token_src = ident_token, - .used = .capture, + .id_cat = .@"capture", }; break :s &payload_val_scope.base; } else { @@ -5056,7 +4990,7 @@ fn ifExpr( .name = ident_name, .inst = payload_inst, .token_src = error_token, - .used = .capture, + .id_cat = .@"capture", }; break :s &payload_val_scope.base; } else { @@ -5250,7 +5184,7 @@ fn whileExpr( .name = ident_name, .inst = payload_inst, .token_src = payload_token, - .used = .capture, + .id_cat = .@"capture", }; break :s &payload_val_scope.base; } else { @@ -5272,7 +5206,7 @@ fn whileExpr( .name = ident_name, .inst = payload_inst, .token_src = ident_token, - .used = .capture, + .id_cat = .@"capture", }; break :s &payload_val_scope.base; } else { @@ -5329,7 +5263,7 @@ fn whileExpr( .name = ident_name, .inst = payload_inst, .token_src = error_token, - .used = .capture, + .id_cat = .@"capture", }; break :s &payload_val_scope.base; } else { @@ -5468,7 +5402,7 @@ fn forExpr( .name = name_str_index, .inst = payload_inst, .token_src = ident, - .used = .capture, + .id_cat = .@"capture", }; payload_sub_scope = &payload_val_scope.base; } else if (is_ptr) { @@ -5492,7 +5426,7 @@ fn forExpr( .ptr = index_ptr, .token_src = index_token, .maybe_comptime = is_inline, - .used = .loop_index, + .id_cat = .@"loop index capture", }; break :blk &index_scope.base; }; @@ -5737,7 +5671,7 @@ fn switchExpr( .name = capture_name, .inst = capture, .token_src = payload_token, - .used = .capture, + .id_cat = .@"capture", }; break :blk &capture_val_scope.base; }; @@ -5831,7 +5765,7 @@ fn switchExpr( .name = capture_name, .inst = capture, .token_src = payload_token, - .used = .capture, + .id_cat = .@"capture", }; break :blk &capture_val_scope.base; }; @@ -6261,7 +6195,7 @@ fn identifier( const local_val = s.cast(Scope.LocalVal).?; if (local_val.name == name_str_index) { - local_val.used = .used; + local_val.used = true; // Captures of non-locals need to be emitted as decl_val or decl_ref. // This *might* be capturable depending on if it is comptime known. if (!hit_namespace) { @@ -6273,7 +6207,7 @@ fn identifier( .local_ptr => { const local_ptr = s.cast(Scope.LocalPtr).?; if (local_ptr.name == name_str_index) { - local_ptr.used = .used; + local_ptr.used = true; if (hit_namespace) { if (local_ptr.maybe_comptime) break @@ -6609,7 +6543,7 @@ fn asmExpr( .local_val => { const local_val = s.cast(Scope.LocalVal).?; if (local_val.name == str_index) { - local_val.used = .used; + local_val.used = true; break; } s = local_val.parent; @@ -6617,7 +6551,7 @@ fn asmExpr( .local_ptr => { const local_ptr = s.cast(Scope.LocalPtr).?; if (local_ptr.name == str_index) { - local_ptr.used = .used; + local_ptr.used = true; break; } s = local_ptr.parent; @@ -6968,7 +6902,7 @@ fn builtinCall( .local_val => { const local_val = s.cast(Scope.LocalVal).?; if (local_val.name == decl_name) { - local_val.used = .used; + local_val.used = true; break; } s = local_val.parent; @@ -6978,7 +6912,7 @@ fn builtinCall( if (local_ptr.name == decl_name) { if (!local_ptr.maybe_comptime) return astgen.failNode(params[0], "unable to export runtime-known value", .{}); - local_ptr.used = .used; + local_ptr.used = true; break; } s = local_ptr.parent; @@ -8545,15 +8479,15 @@ const Scope = struct { top, }; - // either .used or the type of the var/constant - const Used = enum { - fn_param, - constant, - variable, - loop_index, - capture, - used, + /// The category of identifier. These tag names are user-visible in compile errors. + const IdCat = enum { + @"function parameter", + @"local constant", + @"local variable", + @"loop index capture", + @"capture", }; + /// This is always a `const` local and importantly the `inst` is a value type, not a pointer. /// This structure lives as long as the AST generation of the Block /// node that contains the variable. @@ -8568,8 +8502,9 @@ const Scope = struct { token_src: ast.TokenIndex, /// String table index. name: u32, - /// has this variable been referenced? - used: Used, + id_cat: IdCat, + /// Track whether the name has been referenced. + used: bool = false, }; /// This could be a `const` or `var` local. It has a pointer instead of a value. @@ -8586,10 +8521,12 @@ const Scope = struct { token_src: ast.TokenIndex, /// String table index. name: u32, - /// true means we find out during Sema whether the value is comptime. false means it is already known at AstGen the value is runtime-known. + id_cat: IdCat, + /// true means we find out during Sema whether the value is comptime. + /// false means it is already known at AstGen the value is runtime-known. maybe_comptime: bool, - /// has this variable been referenced? - used: Used, + /// Track whether the name has been referenced. + used: bool = false, }; const Defer = struct { @@ -9680,6 +9617,71 @@ fn declareNewName( } } +/// Local variables shadowing detection, including function parameters. +fn detectLocalShadowing( + astgen: *AstGen, + scope: *Scope, + ident_name: u32, + name_token: ast.TokenIndex, +) !void { + const gpa = astgen.gpa; + + var s = scope; + while (true) switch (s.tag) { + .local_val => { + const local_val = s.cast(Scope.LocalVal).?; + if (local_val.name == ident_name) { + const name = try gpa.dupe(u8, mem.spanZ(astgen.nullTerminatedString(ident_name))); + defer gpa.free(name); + return astgen.failTokNotes(name_token, "redeclaration of {s} '{s}'", .{ + @tagName(local_val.id_cat), name, + }, &[_]u32{ + try astgen.errNoteTok( + local_val.token_src, + "previously declared here", + .{}, + ), + }); + } + s = local_val.parent; + }, + .local_ptr => { + const local_ptr = s.cast(Scope.LocalPtr).?; + if (local_ptr.name == ident_name) { + const name = try gpa.dupe(u8, mem.spanZ(astgen.nullTerminatedString(ident_name))); + defer gpa.free(name); + return astgen.failTokNotes(name_token, "redeclaration of {s} '{s}'", .{ + @tagName(local_ptr.id_cat), name, + }, &[_]u32{ + try astgen.errNoteTok( + local_ptr.token_src, + "previously declared here", + .{}, + ), + }); + } + s = local_ptr.parent; + }, + .namespace => { + const ns = s.cast(Scope.Namespace).?; + const decl_node = ns.decls.get(ident_name) orelse { + s = ns.parent; + continue; + }; + const name = try gpa.dupe(u8, mem.spanZ(astgen.nullTerminatedString(ident_name))); + defer gpa.free(name); + return astgen.failTokNotes(name_token, "local shadows declaration of '{s}'", .{ + name, + }, &[_]u32{ + try astgen.errNoteNode(decl_node, "declared here", .{}), + }); + }, + .gen_zir => s = s.cast(GenZir).?.parent, + .defer_normal, .defer_error => s = s.cast(Scope.Defer).?.parent, + .top => break, + }; +} + fn advanceSourceCursor(astgen: *AstGen, source: []const u8, end: usize) void { var i = astgen.source_offset; var line = astgen.source_line;