From 9c95f38a7c1defc7f63a4815bfc2d76a5f9f83f6 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Tue, 24 Aug 2021 18:56:16 -0700 Subject: [PATCH] stage1: remove incorrect compile error for var redeclaration Locals are not allowed to shadow declarations, but declarations are allowed to shadow each other, as long as there are no ambiguous references. closes #678 --- src/stage1/analyze.cpp | 43 ++++------------------------------------ src/stage1/astgen.cpp | 43 +++++++++++++++++++++++----------------- test/behavior/error.zig | 12 +++++------ test/behavior/misc.zig | 17 ++++++++++++++++ test/behavior/struct.zig | 4 ++-- test/compile_errors.zig | 31 ++++++++++++----------------- 6 files changed, 67 insertions(+), 83 deletions(-) diff --git a/src/stage1/analyze.cpp b/src/stage1/analyze.cpp index cf61bf79b7..402c86d86f 100644 --- a/src/stage1/analyze.cpp +++ b/src/stage1/analyze.cpp @@ -4171,46 +4171,11 @@ ZigVar *add_variable(CodeGen *g, AstNode *source_node, Scope *parent_scope, Buf } else { variable_entry->align_bytes = get_abi_alignment(g, var_type); - ZigVar *existing_var = find_variable(g, parent_scope, name, nullptr); - if (existing_var && !existing_var->shadowable) { - if (existing_var->var_type == nullptr || !type_is_invalid(existing_var->var_type)) { - ErrorMsg *msg = add_node_error(g, source_node, - buf_sprintf("redeclaration of variable '%s'", buf_ptr(name))); - add_error_note(g, msg, existing_var->decl_node, buf_sprintf("previous declaration here")); - } + ZigType *type; + if (get_primitive_type(g, name, &type) != ErrorPrimitiveTypeNotFound) { + add_node_error(g, source_node, + buf_sprintf("variable shadows primitive type '%s'", buf_ptr(name))); variable_entry->var_type = g->builtin_types.entry_invalid; - } else { - ZigType *type; - if (get_primitive_type(g, name, &type) != ErrorPrimitiveTypeNotFound) { - add_node_error(g, source_node, - buf_sprintf("variable shadows primitive type '%s'", buf_ptr(name))); - variable_entry->var_type = g->builtin_types.entry_invalid; - } else { - Scope *search_scope = nullptr; - if (src_tld == nullptr) { - search_scope = parent_scope; - } else if (src_tld->parent_scope != nullptr && src_tld->parent_scope->parent != nullptr) { - search_scope = src_tld->parent_scope->parent; - } - if (search_scope != nullptr) { - Tld *tld = find_decl(g, search_scope, name); - if (tld != nullptr && tld != src_tld) { - bool want_err_msg = true; - if (tld->id == TldIdVar) { - ZigVar *var = reinterpret_cast(tld)->var; - if (var != nullptr && var->var_type != nullptr && type_is_invalid(var->var_type)) { - want_err_msg = false; - } - } - if (want_err_msg) { - ErrorMsg *msg = add_node_error(g, source_node, - buf_sprintf("redefinition of '%s'", buf_ptr(name))); - add_error_note(g, msg, tld->source_node, buf_sprintf("previous definition here")); - } - variable_entry->var_type = g->builtin_types.entry_invalid; - } - } - } } } diff --git a/src/stage1/astgen.cpp b/src/stage1/astgen.cpp index e7c4eacb3f..86c18abc1e 100644 --- a/src/stage1/astgen.cpp +++ b/src/stage1/astgen.cpp @@ -3200,23 +3200,6 @@ ZigVar *create_local_var(CodeGen *codegen, AstNode *node, Scope *parent_scope, add_node_error(codegen, node, buf_sprintf("variable shadows primitive type '%s'", buf_ptr(name))); variable_entry->var_type = codegen->builtin_types.entry_invalid; - } else { - Tld *tld = find_decl(codegen, parent_scope, name); - if (tld != nullptr) { - bool want_err_msg = true; - if (tld->id == TldIdVar) { - ZigVar *var = reinterpret_cast(tld)->var; - if (var != nullptr && var->var_type != nullptr && type_is_invalid(var->var_type)) { - want_err_msg = false; - } - } - if (want_err_msg) { - ErrorMsg *msg = add_node_error(codegen, node, - buf_sprintf("redefinition of '%s'", buf_ptr(name))); - add_error_note(codegen, msg, tld->source_node, buf_sprintf("previous definition here")); - } - variable_entry->var_type = codegen->builtin_types.entry_invalid; - } } } } @@ -3875,7 +3858,31 @@ static Stage1ZirInst *astgen_identifier(Stage1AstGen *ag, Scope *scope, AstNode } } - Tld *tld = find_decl(ag->codegen, scope, variable_name); + Tld *tld = nullptr; + { + Scope *s = scope; + while (s) { + if (s->id == ScopeIdDecls) { + ScopeDecls *decls_scope = (ScopeDecls *)s; + + Tld *result = find_container_decl(ag->codegen, decls_scope, variable_name); + if (result != nullptr) { + if (tld != nullptr && tld != result) { + ErrorMsg *msg = add_node_error(ag->codegen, node, + buf_sprintf("ambiguous reference")); + add_error_note(ag->codegen, msg, tld->source_node, + buf_sprintf("declared here")); + add_error_note(ag->codegen, msg, result->source_node, + buf_sprintf("also declared here")); + return ag->codegen->invalid_inst_src; + } + tld = result; + } + } + s = s->parent; + } + } + if (tld) { Stage1ZirInst *decl_ref = ir_build_decl_ref(ag, scope, node, tld, lval); if (lval == LValPtr || lval == LValAssign) { diff --git a/test/behavior/error.zig b/test/behavior/error.zig index b1ef55af9e..74bf105799 100644 --- a/test/behavior/error.zig +++ b/test/behavior/error.zig @@ -412,19 +412,19 @@ test "function pointer with return type that is error union with payload which i test "return result loc as peer result loc in inferred error set function" { const S = struct { fn doTheTest() !void { - if (foo(2)) |x| { + if (quux(2)) |x| { try expect(x.Two); } else |e| switch (e) { error.Whatever => @panic("fail"), } - try expectError(error.Whatever, foo(99)); + try expectError(error.Whatever, quux(99)); } const FormValue = union(enum) { One: void, Two: bool, }; - fn foo(id: u64) !FormValue { + fn quux(id: u64) !FormValue { return switch (id) { 2 => FormValue{ .Two = true }, 1 => FormValue{ .One = {} }, @@ -452,11 +452,11 @@ test "error payload type is correctly resolved" { test "error union comptime caching" { const S = struct { - fn foo(comptime arg: anytype) void { + fn quux(comptime arg: anytype) void { arg catch {}; } }; - S.foo(@as(anyerror!void, {})); - S.foo(@as(anyerror!void, {})); + S.quux(@as(anyerror!void, {})); + S.quux(@as(anyerror!void, {})); } diff --git a/test/behavior/misc.zig b/test/behavior/misc.zig index 466be00bd3..fe53ca2304 100644 --- a/test/behavior/misc.zig +++ b/test/behavior/misc.zig @@ -505,3 +505,20 @@ test "lazy typeInfo value as generic parameter" { }; S.foo(@typeInfo(@TypeOf(.{}))); } + +fn A() type { + return struct { + b: B(), + + const Self = @This(); + + fn B() type { + return struct { + const Self = @This(); + }; + } + }; +} +test "non-ambiguous reference of shadowed decls" { + try expect(A().B().Self != A().Self); +} diff --git a/test/behavior/struct.zig b/test/behavior/struct.zig index 4f7e0c2d71..d9e1c02aa1 100644 --- a/test/behavior/struct.zig +++ b/test/behavior/struct.zig @@ -162,14 +162,14 @@ const MemberFnRand = struct { }; test "return struct byval from function" { - const bar = makeBar(1234, 5678); + const bar = makeBar2(1234, 5678); try expect(bar.y == 5678); } const Bar = struct { x: i32, y: i32, }; -fn makeBar(x: i32, y: i32) Bar { +fn makeBar2(x: i32, y: i32) Bar { return Bar{ .x = x, .y = y, diff --git a/test/compile_errors.zig b/test/compile_errors.zig index 14d2a98e02..ce5c80fa2c 100644 --- a/test/compile_errors.zig +++ b/test/compile_errors.zig @@ -6969,29 +6969,24 @@ pub fn addCases(ctx: *TestContext) !void { "tmp.zig:2:30: error: cannot set section of local variable 'foo'", }); - ctx.objErrStage1("inner struct member shadowing outer struct member", - \\fn A() type { - \\ return struct { - \\ b: B(), - \\ - \\ const Self = @This(); - \\ - \\ fn B() type { - \\ return struct { - \\ const Self = @This(); - \\ }; + ctx.objErrStage1("ambiguous decl reference", + \\fn foo() void {} + \\fn bar() void { + \\ const S = struct { + \\ fn baz() void { + \\ foo(); \\ } + \\ fn foo() void {} \\ }; + \\ S.baz(); \\} - \\comptime { - \\ assert(A().B().Self != A().Self); - \\} - \\fn assert(ok: bool) void { - \\ if (!ok) unreachable; + \\export fn entry() void { + \\ bar(); \\} , &[_][]const u8{ - "tmp.zig:9:17: error: redefinition of 'Self'", - "tmp.zig:5:9: note: previous definition here", + "tmp.zig:5:13: error: ambiguous reference", + "tmp.zig:7:9: note: declared here", + "tmp.zig:1:1: note: also declared here", }); ctx.objErrStage1("while expected bool, got optional",