From af8661405b908c0abfc191501a8ad1a59a54e86a Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Fri, 19 Jul 2019 16:56:44 -0400 Subject: [PATCH] fix usingnamespace It used to be that usingnamespace was only allowed at top level. This made it OK to put the state inside the AST node data structure. However, now usingnamespace can occur inside any aggregate data structure, and therefore the state must be in the TopLevelDeclaration rather than in the AST node. There were two other problems with the usingnamespace implementation: * It was passing the wrong destination ScopeDecl, so it could cause an incorrect error such as "import of file outside package path". * When doing `usingnamespace` on a file that already had `pub usingnamespace` in it would "steal" the usingnamespace, causing incorrect "use of undeclared identifier" errors in the target file. closes #2632 closes #2580 --- src/all_types.hpp | 16 +- src/analyze.cpp | 265 ++++++++++++------------ src/analyze.hpp | 2 - src/ast_render.cpp | 8 +- src/ir.cpp | 3 +- src/parser.cpp | 4 +- test/compile_errors.zig | 2 +- test/stage1/behavior.zig | 1 + test/stage1/behavior/usingnamespace.zig | 14 ++ 9 files changed, 170 insertions(+), 145 deletions(-) create mode 100644 test/stage1/behavior/usingnamespace.zig diff --git a/src/all_types.hpp b/src/all_types.hpp index 79ade39ef..a6b2bc51c 100644 --- a/src/all_types.hpp +++ b/src/all_types.hpp @@ -366,6 +366,7 @@ enum TldId { TldIdFn, TldIdContainer, TldIdCompTime, + TldIdUsingNamespace, }; enum TldResolution { @@ -413,6 +414,12 @@ struct TldCompTime { Tld base; }; +struct TldUsingNamespace { + Tld base; + + ConstExprValue *using_namespace_value; +}; + struct TypeEnumField { Buf *name; BigInt value; @@ -453,7 +460,7 @@ enum NodeType { NodeTypeFieldAccessExpr, NodeTypePtrDeref, NodeTypeUnwrapOptional, - NodeTypeUse, + NodeTypeUsingNamespace, NodeTypeBoolLiteral, NodeTypeNullLiteral, NodeTypeUndefinedLiteral, @@ -715,9 +722,6 @@ struct AstNodeArrayType { struct AstNodeUsingNamespace { VisibMod visib_mod; AstNode *expr; - - TldResolution resolution; - ConstExprValue *using_namespace_value; }; struct AstNodeIfBoolExpr { @@ -1745,8 +1749,6 @@ struct CodeGen { ZigList resolve_queue; size_t resolve_queue_index; - ZigList use_queue; - size_t use_queue_index; ZigList timing_events; ZigList tld_ref_source_node_stack; ZigList inline_fns; @@ -2005,7 +2007,7 @@ struct ScopeDecls { Scope base; HashMap decl_table; - ZigList use_decls; + ZigList use_decls; AstNode *safety_set_node; AstNode *fast_math_set_node; ZigType *import; diff --git a/src/analyze.cpp b/src/analyze.cpp index 797451c8f..de4d64f5d 100644 --- a/src/analyze.cpp +++ b/src/analyze.cpp @@ -28,6 +28,8 @@ static Error ATTRIBUTE_MUST_USE resolve_union_zero_bits(CodeGen *g, ZigType *uni static Error ATTRIBUTE_MUST_USE resolve_union_alignment(CodeGen *g, ZigType *union_type); static void analyze_fn_body(CodeGen *g, ZigFn *fn_table_entry); static void resolve_llvm_types(CodeGen *g, ZigType *type, ResolveStatus wanted_resolve_status); +static void preview_use_decl(CodeGen *g, TldUsingNamespace *using_namespace, ScopeDecls *dest_decls_scope); +static void resolve_use_decl(CodeGen *g, TldUsingNamespace *tld_using_namespace, ScopeDecls *dest_decls_scope); static bool is_top_level_struct(ZigType *import) { return import->id == ZigTypeIdStruct && import->data.structure.root_struct != nullptr; @@ -2854,6 +2856,8 @@ static void add_top_level_decl(CodeGen *g, ScopeDecls *decls_scope, Tld *tld) { add_node_error(g, tld->source_node, buf_sprintf("non-extern function has no body")); return; } + } else if (tld->id == TldIdUsingNamespace) { + g->resolve_queue.append(tld); } if (is_export) { g->resolve_queue.append(tld); @@ -2867,7 +2871,7 @@ static void add_top_level_decl(CodeGen *g, ScopeDecls *decls_scope, Tld *tld) { } } - { + if (tld->name != nullptr) { auto entry = decls_scope->decl_table.put_unique(tld->name, tld); if (entry) { Tld *other_tld = entry->value; @@ -2875,9 +2879,7 @@ 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 is here")); return; } - } - { ZigType *type; if (get_primitive_type(g, tld->name, &type) != ErrorPrimitiveTypeNotFound) { add_node_error(g, tld->source_node, @@ -2977,12 +2979,14 @@ void scan_decls(CodeGen *g, ScopeDecls *decls_scope, AstNode *node) { break; } - case NodeTypeUse: - { - g->use_queue.append(node); - decls_scope->use_decls.append(node); - break; - } + case NodeTypeUsingNamespace: { + VisibMod visib_mod = node->data.using_namespace.visib_mod; + TldUsingNamespace *tld_using_namespace = allocate(1); + init_tld(&tld_using_namespace->base, TldIdUsingNamespace, nullptr, visib_mod, node, &decls_scope->base); + add_top_level_decl(g, decls_scope, &tld_using_namespace->base); + decls_scope->use_decls.append(tld_using_namespace); + break; + } case NodeTypeTestDecl: preview_test_decl(g, node, decls_scope); break; @@ -3266,6 +3270,117 @@ static void resolve_decl_var(CodeGen *g, TldVar *tld_var) { g->global_vars.append(tld_var); } +static void add_symbols_from_container(CodeGen *g, TldUsingNamespace *src_using_namespace, + TldUsingNamespace *dst_using_namespace, ScopeDecls* dest_decls_scope) +{ + if (src_using_namespace->base.resolution == TldResolutionUnresolved || + src_using_namespace->base.resolution == TldResolutionResolving) + { + assert(src_using_namespace->base.parent_scope->id == ScopeIdDecls); + ScopeDecls *src_decls_scope = (ScopeDecls *)src_using_namespace->base.parent_scope; + preview_use_decl(g, src_using_namespace, src_decls_scope); + if (src_using_namespace != dst_using_namespace) { + resolve_use_decl(g, src_using_namespace, src_decls_scope); + } + } + + ConstExprValue *use_expr = src_using_namespace->using_namespace_value; + if (type_is_invalid(use_expr->type)) { + dest_decls_scope->any_imports_failed = true; + return; + } + + dst_using_namespace->base.resolution = TldResolutionOk; + + assert(use_expr->special != ConstValSpecialRuntime); + + // The source scope for the imported symbols + ScopeDecls *src_scope = get_container_scope(use_expr->data.x_type); + // The top-level container where the symbols are defined, it's used in the + // loop below in order to exclude the ones coming from an import statement + ZigType *src_import = get_scope_import(&src_scope->base); + assert(src_import != nullptr); + + if (src_scope->any_imports_failed) { + dest_decls_scope->any_imports_failed = true; + } + + auto it = src_scope->decl_table.entry_iterator(); + for (;;) { + auto *entry = it.next(); + if (!entry) + break; + + Buf *target_tld_name = entry->key; + Tld *target_tld = entry->value; + + if (target_tld->visib_mod == VisibModPrivate) { + continue; + } + + if (target_tld->import != src_import) { + continue; + } + + auto existing_entry = dest_decls_scope->decl_table.put_unique(target_tld_name, target_tld); + if (existing_entry) { + Tld *existing_decl = existing_entry->value; + if (existing_decl != target_tld) { + ErrorMsg *msg = add_node_error(g, dst_using_namespace->base.source_node, + buf_sprintf("import of '%s' overrides existing definition", + buf_ptr(target_tld_name))); + add_error_note(g, msg, existing_decl->source_node, buf_sprintf("previous definition here")); + add_error_note(g, msg, target_tld->source_node, buf_sprintf("imported definition here")); + } + } + } + + for (size_t i = 0; i < src_scope->use_decls.length; i += 1) { + TldUsingNamespace *tld_using_namespace = src_scope->use_decls.at(i); + if (tld_using_namespace->base.visib_mod != VisibModPrivate) + add_symbols_from_container(g, tld_using_namespace, dst_using_namespace, dest_decls_scope); + } +} + +static void resolve_use_decl(CodeGen *g, TldUsingNamespace *tld_using_namespace, ScopeDecls *dest_decls_scope) { + if (tld_using_namespace->base.resolution == TldResolutionOk || + tld_using_namespace->base.resolution == TldResolutionInvalid) + { + return; + } + add_symbols_from_container(g, tld_using_namespace, tld_using_namespace, dest_decls_scope); +} + +static void preview_use_decl(CodeGen *g, TldUsingNamespace *using_namespace, ScopeDecls *dest_decls_scope) { + if (using_namespace->base.resolution == TldResolutionOk || + using_namespace->base.resolution == TldResolutionInvalid) + { + return; + } + + using_namespace->base.resolution = TldResolutionResolving; + assert(using_namespace->base.source_node->type == NodeTypeUsingNamespace); + ConstExprValue *result = analyze_const_value(g, &dest_decls_scope->base, + using_namespace->base.source_node->data.using_namespace.expr, g->builtin_types.entry_type, nullptr); + using_namespace->using_namespace_value = result; + + if (type_is_invalid(result->type)) { + dest_decls_scope->any_imports_failed = true; + using_namespace->base.resolution = TldResolutionInvalid; + using_namespace->using_namespace_value = &g->invalid_instruction->value; + return; + } + + if (!is_container(result->data.x_type)) { + add_node_error(g, using_namespace->base.source_node, + buf_sprintf("expected struct, enum, or union; found '%s'", buf_ptr(&result->data.x_type->name))); + dest_decls_scope->any_imports_failed = true; + using_namespace->base.resolution = TldResolutionInvalid; + using_namespace->using_namespace_value = &g->invalid_instruction->value; + return; + } +} + void resolve_top_level_decl(CodeGen *g, Tld *tld, AstNode *source_node) { if (tld->resolution != TldResolutionUnresolved) return; @@ -3299,6 +3414,14 @@ void resolve_top_level_decl(CodeGen *g, Tld *tld, AstNode *source_node) { resolve_decl_comptime(g, tld_comptime); break; } + case TldIdUsingNamespace: { + TldUsingNamespace *tld_using_namespace = (TldUsingNamespace *)tld; + assert(tld_using_namespace->base.parent_scope->id == ScopeIdDecls); + ScopeDecls *dest_decls_scope = (ScopeDecls *)tld_using_namespace->base.parent_scope; + preview_use_decl(g, tld_using_namespace, dest_decls_scope); + resolve_use_decl(g, tld_using_namespace, dest_decls_scope); + break; + } } tld->resolution = TldResolutionOk; @@ -3308,10 +3431,10 @@ void resolve_top_level_decl(CodeGen *g, Tld *tld, AstNode *source_node) { Tld *find_container_decl(CodeGen *g, ScopeDecls *decls_scope, Buf *name) { // resolve all the using_namespace decls for (size_t i = 0; i < decls_scope->use_decls.length; i += 1) { - AstNode *use_decl_node = decls_scope->use_decls.at(i); - if (use_decl_node->data.using_namespace.resolution == TldResolutionUnresolved) { - preview_use_decl(g, use_decl_node, decls_scope); - resolve_use_decl(g, use_decl_node, decls_scope); + TldUsingNamespace *tld_using_namespace = decls_scope->use_decls.at(i); + if (tld_using_namespace->base.resolution == TldResolutionUnresolved) { + preview_use_decl(g, tld_using_namespace, decls_scope); + resolve_use_decl(g, tld_using_namespace, decls_scope); } } @@ -3752,110 +3875,6 @@ static void analyze_fn_body(CodeGen *g, ZigFn *fn_table_entry) { analyze_fn_ir(g, fn_table_entry, return_type_node); } -static void add_symbols_from_container(CodeGen *g, AstNode *src_use_node, AstNode *dst_use_node, ScopeDecls* decls_scope) { - if (src_use_node->data.using_namespace.resolution == TldResolutionUnresolved) { - preview_use_decl(g, src_use_node, decls_scope); - } - - ConstExprValue *use_expr = src_use_node->data.using_namespace.using_namespace_value; - if (type_is_invalid(use_expr->type)) { - decls_scope->any_imports_failed = true; - return; - } - - dst_use_node->data.using_namespace.resolution = TldResolutionOk; - - assert(use_expr->special != ConstValSpecialRuntime); - - // The source struct for the imported symbols - ZigType *src_ty = use_expr->data.x_type; - assert(src_ty); - - if (!is_container(src_ty)) { - add_node_error(g, dst_use_node, - buf_sprintf("expected struct, enum, or union; found '%s'", buf_ptr(&src_ty->name))); - decls_scope->any_imports_failed = true; - return; - } - - // The source scope for the imported symbols - ScopeDecls *src_scope = get_container_scope(src_ty); - // The top-level container where the symbols are defined, it's used in the - // loop below in order to exclude the ones coming from an import statement - ZigType *src_import = get_scope_import(&src_scope->base); - assert(src_import != nullptr); - - if (src_scope->any_imports_failed) { - decls_scope->any_imports_failed = true; - } - - auto it = src_scope->decl_table.entry_iterator(); - for (;;) { - auto *entry = it.next(); - if (!entry) - break; - - Buf *target_tld_name = entry->key; - Tld *target_tld = entry->value; - - if (target_tld->visib_mod == VisibModPrivate) { - continue; - } - - if (target_tld->import != src_import) { - continue; - } - - auto existing_entry = decls_scope->decl_table.put_unique(target_tld_name, target_tld); - if (existing_entry) { - Tld *existing_decl = existing_entry->value; - if (existing_decl != target_tld) { - ErrorMsg *msg = add_node_error(g, dst_use_node, - buf_sprintf("import of '%s' overrides existing definition", - buf_ptr(target_tld_name))); - add_error_note(g, msg, existing_decl->source_node, buf_sprintf("previous definition here")); - add_error_note(g, msg, target_tld->source_node, buf_sprintf("imported definition here")); - } - } - } - - for (size_t i = 0; i < src_scope->use_decls.length; i += 1) { - AstNode *use_decl_node = src_scope->use_decls.at(i); - if (use_decl_node->data.using_namespace.visib_mod != VisibModPrivate) - add_symbols_from_container(g, use_decl_node, dst_use_node, decls_scope); - } -} - -void resolve_use_decl(CodeGen *g, AstNode *node, ScopeDecls *decls_scope) { - assert(node->type == NodeTypeUse); - - if (node->data.using_namespace.resolution == TldResolutionOk || - node->data.using_namespace.resolution == TldResolutionInvalid) - { - return; - } - add_symbols_from_container(g, node, node, decls_scope); -} - -void preview_use_decl(CodeGen *g, AstNode *node, ScopeDecls *decls_scope) { - assert(node->type == NodeTypeUse); - - if (node->data.using_namespace.resolution == TldResolutionOk || - node->data.using_namespace.resolution == TldResolutionInvalid) - { - return; - } - - node->data.using_namespace.resolution = TldResolutionResolving; - ConstExprValue *result = analyze_const_value(g, &decls_scope->base, - node->data.using_namespace.expr, g->builtin_types.entry_type, nullptr); - - if (type_is_invalid(result->type)) - decls_scope->any_imports_failed = true; - - node->data.using_namespace.using_namespace_value = result; -} - ZigType *add_source_file(CodeGen *g, ZigPackage *package, Buf *resolved_path, Buf *source_code, SourceKind source_kind) { @@ -3975,18 +3994,8 @@ ZigType *add_source_file(CodeGen *g, ZigPackage *package, Buf *resolved_path, Bu void semantic_analyze(CodeGen *g) { while (g->resolve_queue_index < g->resolve_queue.length || - g->fn_defs_index < g->fn_defs.length || - g->use_queue_index < g->use_queue.length) + g->fn_defs_index < g->fn_defs.length) { - for (; g->use_queue_index < g->use_queue.length; g->use_queue_index += 1) { - AstNode *use_decl_node = g->use_queue.at(g->use_queue_index); - // Get the top-level scope where `using_namespace` is used - ScopeDecls *decls_scope = get_container_scope(use_decl_node->owner); - if (use_decl_node->data.using_namespace.resolution == TldResolutionUnresolved) { - preview_use_decl(g, use_decl_node, decls_scope); - resolve_use_decl(g, use_decl_node, decls_scope); - } - } for (; g->resolve_queue_index < g->resolve_queue.length; g->resolve_queue_index += 1) { Tld *tld = g->resolve_queue.at(g->resolve_queue_index); AstNode *source_node = nullptr; diff --git a/src/analyze.hpp b/src/analyze.hpp index a6ad92110..b9e9f2df7 100644 --- a/src/analyze.hpp +++ b/src/analyze.hpp @@ -86,8 +86,6 @@ bool is_array_ref(ZigType *type_entry); bool is_container_ref(ZigType *type_entry); bool is_valid_vector_elem_type(ZigType *elem_type); void scan_decls(CodeGen *g, ScopeDecls *decls_scope, AstNode *node); -void preview_use_decl(CodeGen *g, AstNode *node, ScopeDecls* decls_scope); -void resolve_use_decl(CodeGen *g, AstNode *node, ScopeDecls* decls_scope); ZigFn *scope_fn_entry(Scope *scope); ZigPackage *scope_package(Scope *scope); ZigType *get_scope_import(Scope *scope); diff --git a/src/ast_render.cpp b/src/ast_render.cpp index 154803f88..fe131ab65 100644 --- a/src/ast_render.cpp +++ b/src/ast_render.cpp @@ -193,8 +193,8 @@ static const char *node_type_str(NodeType node_type) { return "Symbol"; case NodeTypePrefixOpExpr: return "PrefixOpExpr"; - case NodeTypeUse: - return "Use"; + case NodeTypeUsingNamespace: + return "UsingNamespace"; case NodeTypeBoolLiteral: return "BoolLiteral"; case NodeTypeNullLiteral: @@ -791,7 +791,7 @@ static void render_node_extra(AstRender *ar, AstNode *node, bool grouped) { AstNode *decls_node = node->data.container_decl.decls.at(decl_i); render_node_grouped(ar, decls_node); - if (decls_node->type == NodeTypeUse || + if (decls_node->type == NodeTypeUsingNamespace || decls_node->type == NodeTypeVariableDeclaration || decls_node->type == NodeTypeFnProto) { @@ -1170,7 +1170,7 @@ static void render_node_extra(AstRender *ar, AstNode *node, bool grouped) { case NodeTypeParamDecl: case NodeTypeTestDecl: case NodeTypeStructField: - case NodeTypeUse: + case NodeTypeUsingNamespace: zig_panic("TODO more ast rendering"); } } diff --git a/src/ir.cpp b/src/ir.cpp index 579875dc3..be7a8e2e5 100644 --- a/src/ir.cpp +++ b/src/ir.cpp @@ -8430,7 +8430,7 @@ static IrInstruction *ir_gen_node_raw(IrBuilder *irb, AstNode *node, Scope *scop switch (node->type) { case NodeTypeStructValueField: case NodeTypeParamDecl: - case NodeTypeUse: + case NodeTypeUsingNamespace: case NodeTypeSwitchProng: case NodeTypeSwitchRange: case NodeTypeStructField: @@ -17847,6 +17847,7 @@ static IrInstruction *ir_analyze_decl_ref(IrAnalyze *ira, IrInstruction *source_ switch (tld->id) { case TldIdContainer: case TldIdCompTime: + case TldIdUsingNamespace: zig_unreachable(); case TldIdVar: { diff --git a/src/parser.cpp b/src/parser.cpp index 0783d4ec1..fe1f89ac9 100644 --- a/src/parser.cpp +++ b/src/parser.cpp @@ -676,7 +676,7 @@ static AstNode *ast_parse_top_level_decl(ParseContext *pc, VisibMod visib_mod) { AstNode *expr = ast_expect(pc, ast_parse_expr); expect_token(pc, TokenIdSemicolon); - AstNode *res = ast_create_node(pc, NodeTypeUse, usingnamespace); + AstNode *res = ast_create_node(pc, NodeTypeUsingNamespace, usingnamespace); res->data.using_namespace.visib_mod = visib_mod; res->data.using_namespace.expr = expr; return res; @@ -2938,7 +2938,7 @@ void ast_visit_node_children(AstNode *node, void (*visit)(AstNode **, void *cont case NodeTypeUnwrapOptional: visit_field(&node->data.unwrap_optional.expr, visit, context); break; - case NodeTypeUse: + case NodeTypeUsingNamespace: visit_field(&node->data.using_namespace.expr, visit, context); break; case NodeTypeBoolLiteral: diff --git a/test/compile_errors.zig b/test/compile_errors.zig index fd365235d..40ce8d304 100644 --- a/test/compile_errors.zig +++ b/test/compile_errors.zig @@ -234,7 +234,7 @@ pub fn addCases(cases: *tests.CompileErrorContext) void { cases.add( "usingnamespace with wrong type", - \\use void; + \\usingnamespace void; , "tmp.zig:1:1: error: expected struct, enum, or union; found 'void'", ); diff --git a/test/stage1/behavior.zig b/test/stage1/behavior.zig index db8fdcf36..71af5586e 100644 --- a/test/stage1/behavior.zig +++ b/test/stage1/behavior.zig @@ -93,6 +93,7 @@ comptime { _ = @import("behavior/undefined.zig"); _ = @import("behavior/underscore.zig"); _ = @import("behavior/union.zig"); + _ = @import("behavior/usingnamespace.zig"); _ = @import("behavior/var_args.zig"); _ = @import("behavior/vector.zig"); _ = @import("behavior/void.zig"); diff --git a/test/stage1/behavior/usingnamespace.zig b/test/stage1/behavior/usingnamespace.zig new file mode 100644 index 000000000..fb45a9392 --- /dev/null +++ b/test/stage1/behavior/usingnamespace.zig @@ -0,0 +1,14 @@ +const std = @import("std"); + +fn Foo(comptime T: type) type { + return struct { + usingnamespace T; + }; +} + +test "usingnamespace inside a generic struct" { + const std2 = Foo(std); + const testing2 = Foo(std.testing); + std2.testing.expect(true); + testing2.expect(true); +}