Commit f30aa25cbf

Andrew Kelley <andrew@ziglang.org>
2021-08-28 06:34:13
declarations may collide with primitives with @"" syntax
* stage2 AstGen: add missing compile error for declaring a local that shadows a primitive. Even with `@""` syntax, it may not have the same name as a primitive. * stage2 AstGen: add a compile error for a global declaration whose name matches a primitive. However it is allowed when using `@""` syntax. * stage1: delete all "declaration shadows primitive" compile errors because they are now handled by stage2 AstGen. * stage1/stage2 AstGen: notice when using `@""` syntax and: - treat `_` as a regular identifier - skip checking if an identifire is a primitive Check the new test cases for clarifications on semantics. closes #6062
1 parent cfae70e
src/stage1/all_types.hpp
@@ -1125,6 +1125,7 @@ struct AstNodeContainerInitExpr {
 
 struct AstNodeIdentifier {
     Buf *name;
+    bool is_at_syntax;
 };
 
 struct AstNodeEnumLiteral {
src/stage1/analyze.cpp
@@ -3918,12 +3918,6 @@ static void add_top_level_decl(CodeGen *g, ScopeDecls *decls_scope, Tld *tld) {
             add_error_note(g, msg, other_tld->source_node, buf_sprintf("previous definition here"));
             return;
         }
-
-        ZigType *type;
-        if (get_primitive_type(g, tld->name, &type) != ErrorPrimitiveTypeNotFound) {
-            add_node_error(g, tld->source_node,
-                buf_sprintf("declaration shadows primitive type '%s'", buf_ptr(tld->name)));
-        }
     }
 }
 
@@ -4170,13 +4164,6 @@ ZigVar *add_variable(CodeGen *g, AstNode *source_node, Scope *parent_scope, Buf
         variable_entry->var_type = g->builtin_types.entry_invalid;
     } else {
         variable_entry->align_bytes = get_abi_alignment(g, var_type);
-
-        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;
-        }
     }
 
     Scope *child_scope;
src/stage1/astgen.cpp
@@ -3194,13 +3194,6 @@ ZigVar *create_local_var(CodeGen *codegen, AstNode *node, Scope *parent_scope,
                     add_error_note(codegen, msg, existing_var->decl_node, buf_sprintf("previous declaration here"));
                 }
                 variable_entry->var_type = codegen->builtin_types.entry_invalid;
-            } else {
-                ZigType *type;
-                if (get_primitive_type(codegen, name, &type) != ErrorPrimitiveTypeNotFound) {
-                    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 {
@@ -3815,35 +3808,38 @@ static Stage1ZirInst *astgen_identifier(Stage1AstGen *ag, Scope *scope, AstNode
     Error err;
     assert(node->type == NodeTypeIdentifier);
 
-    Buf *variable_name = node_identifier_buf(node);
-
-    if (buf_eql_str(variable_name, "_")) {
-        if (lval == LValAssign) {
-            Stage1ZirInstConst *const_instruction = ir_build_instruction<Stage1ZirInstConst>(ag, scope, node);
-            const_instruction->value = ag->codegen->pass1_arena->create<ZigValue>();
-            const_instruction->value->type = get_pointer_to_type(ag->codegen,
-                    ag->codegen->builtin_types.entry_void, false);
-            const_instruction->value->special = ConstValSpecialStatic;
-            const_instruction->value->data.x_ptr.special = ConstPtrSpecialDiscard;
-            return &const_instruction->base;
+    bool is_at_syntax;
+    Buf *variable_name = node_identifier_buf2(node, &is_at_syntax);
+
+    if (!is_at_syntax) {
+        if (buf_eql_str(variable_name, "_")) {
+            if (lval == LValAssign) {
+                Stage1ZirInstConst *const_instruction = ir_build_instruction<Stage1ZirInstConst>(ag, scope, node);
+                const_instruction->value = ag->codegen->pass1_arena->create<ZigValue>();
+                const_instruction->value->type = get_pointer_to_type(ag->codegen,
+                        ag->codegen->builtin_types.entry_void, false);
+                const_instruction->value->special = ConstValSpecialStatic;
+                const_instruction->value->data.x_ptr.special = ConstPtrSpecialDiscard;
+                return &const_instruction->base;
+            }
         }
-    }
 
-    ZigType *primitive_type;
-    if ((err = get_primitive_type(ag->codegen, variable_name, &primitive_type))) {
-        if (err == ErrorOverflow) {
-            add_node_error(ag->codegen, node,
-                buf_sprintf("primitive integer type '%s' exceeds maximum bit width of 65535",
-                    buf_ptr(variable_name)));
-            return ag->codegen->invalid_inst_src;
-        }
-        assert(err == ErrorPrimitiveTypeNotFound);
-    } else {
-        Stage1ZirInst *value = ir_build_const_type(ag, scope, node, primitive_type);
-        if (lval == LValPtr || lval == LValAssign) {
-            return ir_build_ref_src(ag, scope, node, value);
+        ZigType *primitive_type;
+        if ((err = get_primitive_type(ag->codegen, variable_name, &primitive_type))) {
+            if (err == ErrorOverflow) {
+                add_node_error(ag->codegen, node,
+                    buf_sprintf("primitive integer type '%s' exceeds maximum bit width of 65535",
+                        buf_ptr(variable_name)));
+                return ag->codegen->invalid_inst_src;
+            }
+            assert(err == ErrorPrimitiveTypeNotFound);
         } else {
-            return ir_expr_wrap(ag, scope, value, result_loc);
+            Stage1ZirInst *value = ir_build_const_type(ag, scope, node, primitive_type);
+            if (lval == LValPtr || lval == LValAssign) {
+                return ir_build_ref_src(ag, scope, node, value);
+            } else {
+                return ir_expr_wrap(ag, scope, value, result_loc);
+            }
         }
     }
 
src/stage1/parser.cpp
@@ -3482,8 +3482,7 @@ Error source_char_literal(const char *source, uint32_t *result, size_t *bad_inde
     }
 }
 
-
-Buf *token_identifier_buf(RootStruct *root_struct, TokenIndex token) {
+static Buf *token_identifier_buf2(RootStruct *root_struct, TokenIndex token, bool *is_at_syntax) {
     Error err;
     const char *source = buf_ptr(root_struct->source_code);
     size_t byte_offset = root_struct->token_locs[token].offset;
@@ -3495,6 +3494,7 @@ Buf *token_identifier_buf(RootStruct *root_struct, TokenIndex token) {
     assert(source[byte_offset] != '.'); // wrong token index
 
     if (source[byte_offset] == '@') {
+        *is_at_syntax = true;
         size_t bad_index;
         Buf *str = buf_alloc();
         if ((err = source_string_literal_buf(source + byte_offset + 1, str, &bad_index))) {
@@ -3503,6 +3503,7 @@ Buf *token_identifier_buf(RootStruct *root_struct, TokenIndex token) {
         }
         return str;
     } else {
+        *is_at_syntax = false;
         size_t start = byte_offset;
         for (;; byte_offset += 1) {
             if (source[byte_offset] == 0) break;
@@ -3519,7 +3520,17 @@ Buf *token_identifier_buf(RootStruct *root_struct, TokenIndex token) {
     }
 }
 
+Buf *token_identifier_buf(RootStruct *root_struct, TokenIndex token) {
+    bool trash;
+    return token_identifier_buf2(root_struct, token, &trash);
+}
+
 Buf *node_identifier_buf(AstNode *node) {
+    bool trash;
+    return node_identifier_buf2(node, &trash);
+}
+
+Buf *node_identifier_buf2(AstNode *node, bool *is_at_syntax) {
     assert(node->type == NodeTypeIdentifier);
     // Currently, stage1 runs astgen for every comptime function call,
     // resulting the allocation here wasting memory. As a workaround until
@@ -3527,8 +3538,10 @@ Buf *node_identifier_buf(AstNode *node) {
     // we memoize the result into the AST here.
     if (node->data.identifier.name == nullptr) {
         RootStruct *root_struct = node->owner->data.structure.root_struct;
-        node->data.identifier.name = token_identifier_buf(root_struct, node->main_token);
+        node->data.identifier.name = token_identifier_buf2(root_struct, node->main_token,
+                &node->data.identifier.is_at_syntax);
     }
+    *is_at_syntax = node->data.identifier.is_at_syntax;
     return node->data.identifier.name;
 }
 
src/stage1/parser.hpp
@@ -19,6 +19,7 @@ void ast_print(AstNode *node, int indent);
 void ast_visit_node_children(AstNode *node, void (*visit)(AstNode **, void *context), void *context);
 
 Buf *node_identifier_buf(AstNode *node);
+Buf *node_identifier_buf2(AstNode *node, bool *is_at_syntax);
 
 Buf *token_identifier_buf(RootStruct *root_struct, TokenIndex token);
 
src/AstGen.zig
@@ -2891,7 +2891,7 @@ fn fnDecl(
     };
     const fn_name_str_index = try astgen.identAsString(fn_name_token);
 
-    try astgen.declareNewName(scope, fn_name_str_index, decl_node);
+    try astgen.declareNewName(scope, fn_name_str_index, decl_node, fn_name_token);
 
     // We insert this at the beginning so that its instruction index marks the
     // start of the top level declaration.
@@ -3160,7 +3160,7 @@ fn globalVarDecl(
     const name_token = var_decl.ast.mut_token + 1;
     const name_str_index = try astgen.identAsString(name_token);
 
-    try astgen.declareNewName(scope, name_str_index, node);
+    try astgen.declareNewName(scope, name_str_index, node, name_token);
 
     var block_scope: GenZir = .{
         .parent = scope,
@@ -6319,34 +6319,36 @@ fn identifier(
     }
     const ident_name = try astgen.identifierTokenString(ident_token);
 
-    if (simple_types.get(ident_name)) |zir_const_ref| {
-        return rvalue(gz, rl, zir_const_ref, ident);
-    }
+    if (ident_name_raw[0] != '@') {
+        if (simple_types.get(ident_name)) |zir_const_ref| {
+            return rvalue(gz, rl, zir_const_ref, ident);
+        }
 
-    if (ident_name.len >= 2) integer: {
-        const first_c = ident_name[0];
-        if (first_c == 'i' or first_c == 'u') {
-            const signedness: std.builtin.Signedness = switch (first_c == 'i') {
-                true => .signed,
-                false => .unsigned,
-            };
-            const bit_count = std.fmt.parseInt(u16, ident_name[1..], 10) catch |err| switch (err) {
-                error.Overflow => return astgen.failNode(
-                    ident,
-                    "primitive integer type '{s}' exceeds maximum bit width of 65535",
-                    .{ident_name},
-                ),
-                error.InvalidCharacter => break :integer,
-            };
-            const result = try gz.add(.{
-                .tag = .int_type,
-                .data = .{ .int_type = .{
-                    .src_node = gz.nodeIndexToRelative(ident),
-                    .signedness = signedness,
-                    .bit_count = bit_count,
-                } },
-            });
-            return rvalue(gz, rl, result, ident);
+        if (ident_name.len >= 2) integer: {
+            const first_c = ident_name[0];
+            if (first_c == 'i' or first_c == 'u') {
+                const signedness: std.builtin.Signedness = switch (first_c == 'i') {
+                    true => .signed,
+                    false => .unsigned,
+                };
+                const bit_count = std.fmt.parseInt(u16, ident_name[1..], 10) catch |err| switch (err) {
+                    error.Overflow => return astgen.failNode(
+                        ident,
+                        "primitive integer type '{s}' exceeds maximum bit width of 65535",
+                        .{ident_name},
+                    ),
+                    error.InvalidCharacter => break :integer,
+                };
+                const result = try gz.add(.{
+                    .tag = .int_type,
+                    .data = .{ .int_type = .{
+                        .src_node = gz.nodeIndexToRelative(ident),
+                        .signedness = signedness,
+                        .bit_count = bit_count,
+                    } },
+                });
+                return rvalue(gz, rl, result, ident);
+            }
         }
     }
 
@@ -10031,8 +10033,21 @@ fn declareNewName(
     start_scope: *Scope,
     name_index: u32,
     node: ast.Node.Index,
+    name_token: ast.TokenIndex,
 ) !void {
     const gpa = astgen.gpa;
+
+    const token_bytes = astgen.tree.tokenSlice(name_token);
+    if (token_bytes[0] != '@' and isPrimitive(token_bytes)) {
+        return astgen.failTokNotes(name_token, "name shadows primitive '{s}'", .{
+            token_bytes,
+        }, &[_]u32{
+            try astgen.errNoteTok(name_token, "consider using @\"{s}\" to disambiguate", .{
+                token_bytes,
+            }),
+        });
+    }
+
     var scope = start_scope;
     while (true) {
         switch (scope.tag) {
@@ -10060,7 +10075,20 @@ fn declareNewName(
     }
 }
 
-/// Local variables shadowing detection, including function parameters.
+fn isPrimitive(name: []const u8) bool {
+    if (simple_types.get(name) != null) return true;
+    if (name.len < 2) return false;
+    const first_c = name[0];
+    if (first_c != 'i' and first_c != 'u') return false;
+    if (std.fmt.parseInt(u16, name[1..], 10)) |_| {
+        return true;
+    } else |err| switch (err) {
+        error.Overflow => return true,
+        error.InvalidCharacter => return false,
+    }
+}
+
+/// Local variables shadowing detection, including function parameters and primitives.
 fn detectLocalShadowing(
     astgen: *AstGen,
     scope: *Scope,
@@ -10068,13 +10096,19 @@ fn detectLocalShadowing(
     name_token: ast.TokenIndex,
 ) !void {
     const gpa = astgen.gpa;
+    const name_slice = mem.spanZ(astgen.nullTerminatedString(ident_name));
+    if (isPrimitive(name_slice)) {
+        const name = try gpa.dupe(u8, name_slice);
+        defer gpa.free(name);
+        return astgen.failTok(name_token, "local shadows primitive '{s}'", .{name});
+    }
 
     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)));
+                const name = try gpa.dupe(u8, name_slice);
                 defer gpa.free(name);
                 return astgen.failTokNotes(name_token, "redeclaration of {s} '{s}'", .{
                     @tagName(local_val.id_cat), name,
@@ -10091,7 +10125,7 @@ fn detectLocalShadowing(
         .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)));
+                const name = try gpa.dupe(u8, name_slice);
                 defer gpa.free(name);
                 return astgen.failTokNotes(name_token, "redeclaration of {s} '{s}'", .{
                     @tagName(local_ptr.id_cat), name,
@@ -10111,7 +10145,7 @@ fn detectLocalShadowing(
                 s = ns.parent;
                 continue;
             };
-            const name = try gpa.dupe(u8, mem.spanZ(astgen.nullTerminatedString(ident_name)));
+            const name = try gpa.dupe(u8, name_slice);
             defer gpa.free(name);
             return astgen.failTokNotes(name_token, "local shadows declaration of '{s}'", .{
                 name,
test/behavior/misc.zig
@@ -522,3 +522,15 @@ fn A() type {
 test "non-ambiguous reference of shadowed decls" {
     try expect(A().B().Self != A().Self);
 }
+
+test "use of declaration with same name as primitive" {
+    const S = struct {
+        const @"u8" = u16;
+        const alias = @"u8";
+    };
+    const a: S.u8 = 300;
+    try expect(a == 300);
+
+    const b: S.alias = 300;
+    try expect(b == 300);
+}
test/compile_errors.zig
@@ -7258,14 +7258,41 @@ pub fn addCases(ctx: *TestContext) !void {
         "tmp.zig:2:17: error: expected type 'u3', found 'u8'",
     });
 
-    ctx.objErrStage1("globally shadowing a primitive type",
-        \\const u16 = u8;
+    ctx.objErrStage1("locally shadowing a primitive type",
+        \\export fn foo() void {
+        \\    const u8 = u16;
+        \\    const a: u8 = 300;
+        \\    _ = a;
+        \\}
+        \\export fn bar() void {
+        \\    const @"u8" = u16;
+        \\    const a: @"u8" = 300;
+        \\    _ = a;
+        \\}
+    , &[_][]const u8{
+        "tmp.zig:2:11: error: local shadows primitive 'u8'",
+        "tmp.zig:7:11: error: local shadows primitive 'u8'",
+    });
+
+    ctx.objErrStage1("primitives take precedence over declarations",
+        \\const @"u8" = u16;
+        \\export fn entry() void {
+        \\    const a: u8 = 300;
+        \\    _ = a;
+        \\}
+    , &[_][]const u8{
+        "tmp.zig:3:19: error: integer value 300 cannot be coerced to type 'u8'",
+    });
+
+    ctx.objErrStage1("declaration with same name as primitive must use special syntax",
+        \\const u8 = u16;
         \\export fn entry() void {
-        \\    const a: u16 = 300;
+        \\    const a: u8 = 300;
         \\    _ = a;
         \\}
     , &[_][]const u8{
-        "tmp.zig:1:1: error: declaration shadows primitive type 'u16'",
+        "tmp.zig:1:7: error: name shadows primitive 'u8'",
+        "tmp.zig:1:7: note: consider using @\"u8\" to disambiguate",
     });
 
     ctx.objErrStage1("implicitly increasing pointer alignment",