From 84700e09c9903e5e40badb829b599dd67886f68c Mon Sep 17 00:00:00 2001 From: Feoramund <161657516+Feoramund@users.noreply.github.com> Date: Fri, 20 Sep 2024 19:02:00 -0400 Subject: Forbid parsing more fields if no separator was found Fixes #4278 --- src/parser.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'src/parser.cpp') diff --git a/src/parser.cpp b/src/parser.cpp index 520a23c5a..5b4604c6d 100644 --- a/src/parser.cpp +++ b/src/parser.cpp @@ -4377,10 +4377,14 @@ gb_internal Ast *parse_field_list(AstFile *f, isize *name_count_, u32 allowed_fl } } - allow_field_separator(f); + bool more_fields = allow_field_separator(f); Ast *param = ast_field(f, names, type, default_value, set_flags, tag, docs, f->line_comment); array_add(¶ms, param); + if (!more_fields) { + if (name_count_) *name_count_ = total_name_count; + return ast_field_list(f, start_token, params); + } while (f->curr_token.kind != follow && f->curr_token.kind != Token_EOF && -- cgit v1.2.3 From a7d7c92a5302a9d0db503af37fe96c737a536544 Mon Sep 17 00:00:00 2001 From: gingerBill Date: Mon, 30 Sep 2024 13:05:28 +0100 Subject: `#min_field_align` & `#max_field_align`; deprecate `#field_align` in favour of `#min_field_align` --- core/odin/ast/ast.odin | 23 ++++++++++---------- core/odin/ast/clone.odin | 3 ++- core/odin/parser/parser.odin | 43 ++++++++++++++++++++++++------------- src/check_type.cpp | 31 +++++++++++++++++++++------ src/parser.cpp | 50 ++++++++++++++++++++++++++++++++++---------- src/parser.hpp | 3 ++- src/types.cpp | 12 +++++++---- 7 files changed, 116 insertions(+), 49 deletions(-) (limited to 'src/parser.cpp') diff --git a/core/odin/ast/ast.odin b/core/odin/ast/ast.odin index d67eb31f3..f62feec8c 100644 --- a/core/odin/ast/ast.odin +++ b/core/odin/ast/ast.odin @@ -776,17 +776,18 @@ Dynamic_Array_Type :: struct { Struct_Type :: struct { using node: Expr, - tok_pos: tokenizer.Pos, - poly_params: ^Field_List, - align: ^Expr, - field_align: ^Expr, - where_token: tokenizer.Token, - where_clauses: []^Expr, - is_packed: bool, - is_raw_union: bool, - is_no_copy: bool, - fields: ^Field_List, - name_count: int, + tok_pos: tokenizer.Pos, + poly_params: ^Field_List, + align: ^Expr, + min_field_align: ^Expr, + max_field_align: ^Expr, + where_token: tokenizer.Token, + where_clauses: []^Expr, + is_packed: bool, + is_raw_union: bool, + is_no_copy: bool, + fields: ^Field_List, + name_count: int, } Union_Type_Kind :: enum u8 { diff --git a/core/odin/ast/clone.odin b/core/odin/ast/clone.odin index b0a1673b2..67f7ffa95 100644 --- a/core/odin/ast/clone.odin +++ b/core/odin/ast/clone.odin @@ -316,7 +316,8 @@ clone_node :: proc(node: ^Node) -> ^Node { case ^Struct_Type: r.poly_params = auto_cast clone(r.poly_params) r.align = clone(r.align) - r.field_align = clone(r.field_align) + r.min_field_align = clone(r.min_field_align) + r.max_field_align = clone(r.max_field_align) r.fields = auto_cast clone(r.fields) case ^Union_Type: r.poly_params = auto_cast clone(r.poly_params) diff --git a/core/odin/parser/parser.odin b/core/odin/parser/parser.odin index d42766bde..32246be3a 100644 --- a/core/odin/parser/parser.odin +++ b/core/odin/parser/parser.odin @@ -2610,9 +2610,10 @@ parse_operand :: proc(p: ^Parser, lhs: bool) -> ^ast.Expr { case .Struct: tok := expect_token(p, .Struct) - poly_params: ^ast.Field_List - align: ^ast.Expr - field_align: ^ast.Expr + poly_params: ^ast.Field_List + align: ^ast.Expr + min_field_align: ^ast.Expr + max_field_align: ^ast.Expr is_packed: bool is_raw_union: bool is_no_copy: bool @@ -2645,10 +2646,21 @@ parse_operand :: proc(p: ^Parser, lhs: bool) -> ^ast.Expr { } align = parse_expr(p, true) case "field_align": - if field_align != nil { + if min_field_align != nil { + error(p, tag.pos, "duplicate struct tag '#%s'", tag.text) + } + warn(p, tag.pos, "#field_align has been deprecated in favour of #min_field_align") + min_field_align = parse_expr(p, true) + case "min_field_align": + if min_field_align != nil { + error(p, tag.pos, "duplicate struct tag '#%s'", tag.text) + } + min_field_align = parse_expr(p, true) + case "max_field_align": + if max_field_align != nil { error(p, tag.pos, "duplicate struct tag '#%s'", tag.text) } - field_align = parse_expr(p, true) + max_field_align = parse_expr(p, true) case "raw_union": if is_raw_union { error(p, tag.pos, "duplicate struct tag '#%s'", tag.text) @@ -2689,16 +2701,17 @@ parse_operand :: proc(p: ^Parser, lhs: bool) -> ^ast.Expr { close := expect_closing_brace_of_field_list(p) st := ast.new(ast.Struct_Type, tok.pos, end_pos(close)) - st.poly_params = poly_params - st.align = align - st.field_align = field_align - st.is_packed = is_packed - st.is_raw_union = is_raw_union - st.is_no_copy = is_no_copy - st.fields = fields - st.name_count = name_count - st.where_token = where_token - st.where_clauses = where_clauses + st.poly_params = poly_params + st.align = align + st.min_field_align = min_field_align + st.max_field_align = max_field_align + st.is_packed = is_packed + st.is_raw_union = is_raw_union + st.is_no_copy = is_no_copy + st.fields = fields + st.name_count = name_count + st.where_token = where_token + st.where_clauses = where_clauses return st case .Union: diff --git a/src/check_type.cpp b/src/check_type.cpp index f0e0acb9b..bbeff9ca7 100644 --- a/src/check_type.cpp +++ b/src/check_type.cpp @@ -673,7 +673,7 @@ gb_internal void check_struct_type(CheckerContext *ctx, Type *struct_type, Ast * #define ST_ALIGN(_name) if (st->_name != nullptr) { \ if (st->is_packed) { \ - syntax_error(st->_name, "'#%s' cannot be applied with '#packed'", #_name); \ + error(st->_name, "'#%s' cannot be applied with '#packed'", #_name); \ return; \ } \ i64 align = 1; \ @@ -682,12 +682,31 @@ gb_internal void check_struct_type(CheckerContext *ctx, Type *struct_type, Ast * } \ } - ST_ALIGN(field_align); + ST_ALIGN(min_field_align); + ST_ALIGN(max_field_align); ST_ALIGN(align); - if (struct_type->Struct.custom_align < struct_type->Struct.custom_field_align) { - warning(st->align, "#align(%lld) is defined to be less than #field_name(%lld)", - cast(long long)struct_type->Struct.custom_align, - cast(long long)struct_type->Struct.custom_field_align); + if (struct_type->Struct.custom_align < struct_type->Struct.custom_min_field_align) { + error(st->align, "#align(%lld) is defined to be less than #min_field_align(%lld)", + cast(long long)struct_type->Struct.custom_align, + cast(long long)struct_type->Struct.custom_min_field_align); + } + if (struct_type->Struct.custom_max_field_align != 0 && + struct_type->Struct.custom_align > struct_type->Struct.custom_max_field_align) { + error(st->align, "#align(%lld) is defined to be greater than #max_field_align(%lld)", + cast(long long)struct_type->Struct.custom_align, + cast(long long)struct_type->Struct.custom_max_field_align); + } + if (struct_type->Struct.custom_max_field_align != 0 && + struct_type->Struct.custom_min_field_align > struct_type->Struct.custom_max_field_align) { + error(st->align, "#min_field_align(%lld) is defined to be greater than #max_field_align(%lld)", + cast(long long)struct_type->Struct.custom_min_field_align, + cast(long long)struct_type->Struct.custom_max_field_align); + + i64 a = gb_min(struct_type->Struct.custom_min_field_align, struct_type->Struct.custom_max_field_align); + i64 b = gb_max(struct_type->Struct.custom_min_field_align, struct_type->Struct.custom_max_field_align); + // NOTE(bill): sort them to keep code consistent + struct_type->Struct.custom_min_field_align = a; + struct_type->Struct.custom_max_field_align = b; } #undef ST_ALIGN diff --git a/src/parser.cpp b/src/parser.cpp index 5b4604c6d..4029d49de 100644 --- a/src/parser.cpp +++ b/src/parser.cpp @@ -448,7 +448,8 @@ gb_internal Ast *clone_ast(Ast *node, AstFile *f) { n->StructType.fields = clone_ast_array(n->StructType.fields, f); n->StructType.polymorphic_params = clone_ast(n->StructType.polymorphic_params, f); n->StructType.align = clone_ast(n->StructType.align, f); - n->StructType.field_align = clone_ast(n->StructType.field_align, f); + n->StructType.min_field_align = clone_ast(n->StructType.min_field_align, f); + n->StructType.max_field_align = clone_ast(n->StructType.max_field_align, f); n->StructType.where_clauses = clone_ast_array(n->StructType.where_clauses, f); break; case Ast_UnionType: @@ -1217,7 +1218,7 @@ gb_internal Ast *ast_dynamic_array_type(AstFile *f, Token token, Ast *elem) { gb_internal Ast *ast_struct_type(AstFile *f, Token token, Slice fields, isize field_count, Ast *polymorphic_params, bool is_packed, bool is_raw_union, bool is_no_copy, - Ast *align, Ast *field_align, + Ast *align, Ast *min_field_align, Ast *max_field_align, Token where_token, Array const &where_clauses) { Ast *result = alloc_ast_node(f, Ast_StructType); result->StructType.token = token; @@ -1228,7 +1229,8 @@ gb_internal Ast *ast_struct_type(AstFile *f, Token token, Slice fields, i result->StructType.is_raw_union = is_raw_union; result->StructType.is_no_copy = is_no_copy; result->StructType.align = align; - result->StructType.field_align = field_align; + result->StructType.min_field_align = min_field_align; + result->StructType.max_field_align = max_field_align; result->StructType.where_token = where_token; result->StructType.where_clauses = slice_from_array(where_clauses); return result; @@ -2757,7 +2759,8 @@ gb_internal Ast *parse_operand(AstFile *f, bool lhs) { bool is_raw_union = false; bool no_copy = false; Ast *align = nullptr; - Ast *field_align = nullptr; + Ast *min_field_align = nullptr; + Ast *max_field_align = nullptr; if (allow_token(f, Token_OpenParen)) { isize param_count = 0; @@ -2795,18 +2798,43 @@ gb_internal Ast *parse_operand(AstFile *f, bool lhs) { gb_string_free(s); } } else if (tag.string == "field_align") { - if (field_align) { + if (min_field_align) { syntax_error(tag, "Duplicate struct tag '#%.*s'", LIT(tag.string)); } - field_align = parse_expr(f, true); - if (field_align && field_align->kind != Ast_ParenExpr) { + syntax_warning(tag, "#field_align has been deprecated in favour of #min_field_align"); + min_field_align = parse_expr(f, true); + if (min_field_align && min_field_align->kind != Ast_ParenExpr) { ERROR_BLOCK(); - gbString s = expr_to_string(field_align); + gbString s = expr_to_string(min_field_align); syntax_warning(tag, "#field_align requires parentheses around the expression"); - error_line("\tSuggestion: #field_align(%s)", s); + error_line("\tSuggestion: #min_field_align(%s)", s); gb_string_free(s); } - } else if (tag.string == "raw_union") { + } else if (tag.string == "min_field_align") { + if (min_field_align) { + syntax_error(tag, "Duplicate struct tag '#%.*s'", LIT(tag.string)); + } + min_field_align = parse_expr(f, true); + if (min_field_align && min_field_align->kind != Ast_ParenExpr) { + ERROR_BLOCK(); + gbString s = expr_to_string(min_field_align); + syntax_warning(tag, "#min_field_align requires parentheses around the expression"); + error_line("\tSuggestion: #min_field_align(%s)", s); + gb_string_free(s); + } + } else if (tag.string == "max_field_align") { + if (max_field_align) { + syntax_error(tag, "Duplicate struct tag '#%.*s'", LIT(tag.string)); + } + max_field_align = parse_expr(f, true); + if (max_field_align && max_field_align->kind != Ast_ParenExpr) { + ERROR_BLOCK(); + gbString s = expr_to_string(max_field_align); + syntax_warning(tag, "#max_field_align requires parentheses around the expression"); + error_line("\tSuggestion: #max_field_align(%s)", s); + gb_string_free(s); + } + }else if (tag.string == "raw_union") { if (is_raw_union) { syntax_error(tag, "Duplicate struct tag '#%.*s'", LIT(tag.string)); } @@ -2856,7 +2884,7 @@ gb_internal Ast *parse_operand(AstFile *f, bool lhs) { parser_check_polymorphic_record_parameters(f, polymorphic_params); - return ast_struct_type(f, token, decls, name_count, polymorphic_params, is_packed, is_raw_union, no_copy, align, field_align, where_token, where_clauses); + return ast_struct_type(f, token, decls, name_count, polymorphic_params, is_packed, is_raw_union, no_copy, align, min_field_align, max_field_align, where_token, where_clauses); } break; case Token_union: { diff --git a/src/parser.hpp b/src/parser.hpp index d5117a31e..e332fed50 100644 --- a/src/parser.hpp +++ b/src/parser.hpp @@ -736,7 +736,8 @@ AST_KIND(_TypeBegin, "", bool) \ isize field_count; \ Ast *polymorphic_params; \ Ast *align; \ - Ast *field_align; \ + Ast *min_field_align; \ + Ast *max_field_align; \ Token where_token; \ Slice where_clauses; \ bool is_packed; \ diff --git a/src/types.cpp b/src/types.cpp index a9a7d6dda..ec9917506 100644 --- a/src/types.cpp +++ b/src/types.cpp @@ -137,7 +137,8 @@ struct TypeStruct { Scope * scope; i64 custom_align; - i64 custom_field_align; + i64 custom_min_field_align; + i64 custom_max_field_align; Type * polymorphic_params; // Type_Tuple Type * polymorphic_parent; Wait_Signal polymorphic_wait_signal; @@ -3950,7 +3951,7 @@ gb_internal i64 type_align_of_internal(Type *t, TypePath *path) { return gb_clamp(next_pow2(type_size_of_internal(t, path)), 1, build_context.max_align); } -gb_internal i64 *type_set_offsets_of(Slice const &fields, bool is_packed, bool is_raw_union, i64 min_field_align) { +gb_internal i64 *type_set_offsets_of(Slice const &fields, bool is_packed, bool is_raw_union, i64 min_field_align, i64 max_field_align) { gbAllocator a = permanent_allocator(); auto offsets = gb_alloc_array(a, i64, fields.count); i64 curr_offset = 0; @@ -3980,6 +3981,9 @@ gb_internal i64 *type_set_offsets_of(Slice const &fields, bool is_pack } else { Type *t = fields[i]->type; i64 align = gb_max(type_align_of(t), min_field_align); + if (max_field_align > min_field_align) { + align = gb_min(align, max_field_align); + } i64 size = gb_max(type_size_of( t), 0); curr_offset = align_formula(curr_offset, align); offsets[i] = curr_offset; @@ -3996,7 +4000,7 @@ gb_internal bool type_set_offsets(Type *t) { MUTEX_GUARD(&t->Struct.offset_mutex); if (!t->Struct.are_offsets_set) { t->Struct.are_offsets_being_processed = true; - t->Struct.offsets = type_set_offsets_of(t->Struct.fields, t->Struct.is_packed, t->Struct.is_raw_union, t->Struct.custom_field_align); + t->Struct.offsets = type_set_offsets_of(t->Struct.fields, t->Struct.is_packed, t->Struct.is_raw_union, t->Struct.custom_min_field_align, t->Struct.custom_max_field_align); t->Struct.are_offsets_being_processed = false; t->Struct.are_offsets_set = true; return true; @@ -4005,7 +4009,7 @@ gb_internal bool type_set_offsets(Type *t) { MUTEX_GUARD(&t->Tuple.mutex); if (!t->Tuple.are_offsets_set) { t->Tuple.are_offsets_being_processed = true; - t->Tuple.offsets = type_set_offsets_of(t->Tuple.variables, t->Tuple.is_packed, false, 1); + t->Tuple.offsets = type_set_offsets_of(t->Tuple.variables, t->Tuple.is_packed, false, 1, 0); t->Tuple.are_offsets_being_processed = false; t->Tuple.are_offsets_set = true; return true; -- cgit v1.2.3 From b59647084b11a7f08ad3aa54ae47ceb157f12c46 Mon Sep 17 00:00:00 2001 From: bobsayshilol Date: Sun, 27 Oct 2024 20:26:34 +0000 Subject: Plug a memory leak The call to |array_make()| always allocates and since this variable was unused it lead to a leak. Simply plug it by removing it. --- src/parser.cpp | 2 -- 1 file changed, 2 deletions(-) (limited to 'src/parser.cpp') diff --git a/src/parser.cpp b/src/parser.cpp index 4029d49de..9d8b0d231 100644 --- a/src/parser.cpp +++ b/src/parser.cpp @@ -4262,8 +4262,6 @@ gb_internal bool allow_field_separator(AstFile *f) { gb_internal Ast *parse_struct_field_list(AstFile *f, isize *name_count_) { Token start_token = f->curr_token; - auto decls = array_make(ast_allocator(f)); - isize total_name_count = 0; Ast *params = parse_field_list(f, &total_name_count, FieldFlag_Struct, Token_CloseBrace, false, false); -- cgit v1.2.3 From b3d1d7b835fa5b05620f91420838e4dcdee65dfd Mon Sep 17 00:00:00 2001 From: gingerBill Date: Thu, 14 Nov 2024 16:08:53 +0000 Subject: Make `#relative` types an error in parsing --- src/parser.cpp | 1 + 1 file changed, 1 insertion(+) (limited to 'src/parser.cpp') diff --git a/src/parser.cpp b/src/parser.cpp index 9d8b0d231..aa90651d3 100644 --- a/src/parser.cpp +++ b/src/parser.cpp @@ -2488,6 +2488,7 @@ gb_internal Ast *parse_operand(AstFile *f, bool lhs) { tag = parse_call_expr(f, tag); } Ast *type = parse_type(f); + syntax_error(tag, "#relative types have now been removed in favour of \"core:relative\""); return ast_relative_type(f, tag, type); } else if (name.string == "force_inline" || name.string == "force_no_inline") { -- cgit v1.2.3