authorgravatar for robin@voetter.nlRobin Voetter <robin@voetter.nl> 2021-10-19 15:08:28+02:00
committergravatar for robin@voetter.nlRobin Voetter <robin@voetter.nl> 2021-10-20 03:44:02+02:00
logb65582e834de34f2351fa04a47f20e7f9c16a47c
tree903977d19050915584c568b9a6b04b57e0996d59
parentbfedf40c9267367d6ffdad54050dcbd451136b58

stage2: remove AstGen none_or_ref

The remaining uses of this result location were causing a bunch of errors problems where the pointers returned from rvalue and lvalue expressions would be confused, allowing for extra pointers on rvalue expressions. For example: ```zig const X = struct {a: i32}; var x: X = .{.a = 1}; var ptr = &x; _ = x.a; ``` In the last line, the lookup of x with result location .none_or_ref would return a double pointer (**X). This would be dereferenced one, after which a relative pointer to `a` would be fetched and derefenced to get the final result. However, this also allows us to manually construct a double pointer, and fetch the field of the inner type of that: ```zig _ = &(&(x)).a; ``` This problem also manifests itself with element access. There are two obvious ways to fix the problem, both of which include replacing the usage of .none_or_ref for field- and element accesses with something which deterministically produce either a pointer or value: either result location .ref or .none. In the former case, this would be paired with .elem_ptr, and in the latter case with .elem_val. Note that the stage 1 compiler does not have this problem, because there is no equivalent of .elem_val and .field_val. In this way it is equivalent to using the result location .ref for field- and element accesses. In this case i have used .none, as this matches language behaviour more closely.

1 files changed, 16 insertions(+), 21 deletions(-)

src/AstGen.zig+16-21
......@@ -193,9 +193,6 @@ pub const ResultLoc = union(enum) {
193193 /// The expression must generate a pointer rather than a value. For example, the left hand side
194194 /// of an assignment uses this kind of result location.
195195 ref,
196 /// The callee will accept a ref, but it is not necessary, and the `ResultLoc`
197 /// may be treated as `none` instead.
198 none_or_ref,
199196 /// The expression will be coerced into this type, but it will be evaluated as an rvalue.
200197 ty: Zir.Inst.Ref,
201198 /// Same as `ty` but it is guaranteed that Sema will additionally perform the coercion,
......@@ -231,7 +228,7 @@ pub const ResultLoc = union(enum) {
231228 fn strategy(rl: ResultLoc, block_scope: *GenZir) Strategy {
232229 switch (rl) {
233230 // In this branch there will not be any store_to_block_ptr instructions.
234 .discard, .none, .none_or_ref, .ty, .coerced_ty, .ref => return .{
231 .discard, .none, .ty, .coerced_ty, .ref => return .{
235232 .tag = .break_operand,
236233 .elide_store_to_block_ptr_instructions = false,
237234 },
......@@ -727,7 +724,7 @@ fn expr(gz: *GenZir, scope: *Scope, rl: ResultLoc, node: Ast.Node.Index) InnerEr
727724 .start = start,
728725 });
729726 switch (rl) {
730 .ref, .none_or_ref => return result,
727 .ref => return result,
731728 else => {
732729 const dereffed = try gz.addUnNode(.load, result, node);
733730 return rvalue(gz, rl, dereffed, node);
......@@ -745,7 +742,7 @@ fn expr(gz: *GenZir, scope: *Scope, rl: ResultLoc, node: Ast.Node.Index) InnerEr
745742 .end = end,
746743 });
747744 switch (rl) {
748 .ref, .none_or_ref => return result,
745 .ref => return result,
749746 else => {
750747 const dereffed = try gz.addUnNode(.load, result, node);
751748 return rvalue(gz, rl, dereffed, node);
......@@ -765,7 +762,7 @@ fn expr(gz: *GenZir, scope: *Scope, rl: ResultLoc, node: Ast.Node.Index) InnerEr
765762 .sentinel = sentinel,
766763 });
767764 switch (rl) {
768 .ref, .none_or_ref => return result,
765 .ref => return result,
769766 else => {
770767 const dereffed = try gz.addUnNode(.load, result, node);
771768 return rvalue(gz, rl, dereffed, node);
......@@ -776,7 +773,7 @@ fn expr(gz: *GenZir, scope: *Scope, rl: ResultLoc, node: Ast.Node.Index) InnerEr
776773 .deref => {
777774 const lhs = try expr(gz, scope, .none, node_datas[node].lhs);
778775 switch (rl) {
779 .ref, .none_or_ref => return lhs,
776 .ref => return lhs,
780777 else => {
781778 const result = try gz.addUnNode(.load, lhs, node);
782779 return rvalue(gz, rl, result, node);
......@@ -1273,7 +1270,7 @@ fn arrayInitExpr(
12731270 return arrayInitExprRlNone(gz, scope, node, array_init.ast.elements, .array_init_anon_ref);
12741271 }
12751272 },
1276 .none, .none_or_ref => {
1273 .none => {
12771274 if (types.array != .none) {
12781275 return arrayInitExprRlTy(gz, scope, node, array_init.ast.elements, types.elem, .array_init);
12791276 } else {
......@@ -1475,7 +1472,7 @@ fn structInitExpr(
14751472 return structInitExprRlNone(gz, scope, node, struct_init, .struct_init_anon_ref);
14761473 }
14771474 },
1478 .none, .none_or_ref => {
1475 .none => {
14791476 if (struct_init.ast.type_expr != 0) {
14801477 const ty_inst = try typeExpr(gz, scope, struct_init.ast.type_expr);
14811478 return structInitExprRlTy(gz, scope, node, struct_init, ty_inst, .struct_init);
......@@ -5133,7 +5130,7 @@ fn fieldAccess(
51335130 if (rl == .ref) {
51345131 return addFieldAccess(.field_ptr, gz, scope, .ref, node);
51355132 } else {
5136 const access = try addFieldAccess(.field_val, gz, scope, .none_or_ref, node);
5133 const access = try addFieldAccess(.field_val, gz, scope, .none, node);
51375134 return rvalue(gz, rl, access, node);
51385135 }
51395136}
......@@ -5178,7 +5175,7 @@ fn arrayAccess(
51785175 ),
51795176 else => return rvalue(gz, rl, try gz.addBin(
51805177 .elem_val,
5181 try expr(gz, scope, .none_or_ref, node_datas[node].lhs),
5178 try expr(gz, scope, .none, node_datas[node].lhs),
51825179 try expr(gz, scope, .{ .ty = .usize_type }, node_datas[node].rhs),
51835180 ), node),
51845181 }
......@@ -6664,7 +6661,7 @@ fn identifier(
66646661 );
66656662
66666663 switch (rl) {
6667 .ref, .none_or_ref => return ptr_inst,
6664 .ref => return ptr_inst,
66686665 else => {
66696666 const loaded = try gz.addUnNode(.load, ptr_inst, ident);
66706667 return rvalue(gz, rl, loaded, ident);
......@@ -6700,7 +6697,7 @@ fn identifier(
67006697 // Decl references happen by name rather than ZIR index so that when unrelated
67016698 // decls are modified, ZIR code containing references to them can be unmodified.
67026699 switch (rl) {
6703 .ref, .none_or_ref => return gz.addStrTok(.decl_ref, name_str_index, ident_token),
6700 .ref => return gz.addStrTok(.decl_ref, name_str_index, ident_token),
67046701 else => {
67056702 const result = try gz.addStrTok(.decl_val, name_str_index, ident_token);
67066703 return rvalue(gz, rl, result, ident);
......@@ -7105,7 +7102,7 @@ fn as(
71057102) InnerError!Zir.Inst.Ref {
71067103 const dest_type = try typeExpr(gz, scope, lhs);
71077104 switch (rl) {
7108 .none, .none_or_ref, .discard, .ref, .ty, .coerced_ty => {
7105 .none, .discard, .ref, .ty, .coerced_ty => {
71097106 const result = try reachableExpr(gz, scope, .{ .ty = dest_type }, rhs, node);
71107107 return rvalue(gz, rl, result, node);
71117108 },
......@@ -7128,7 +7125,7 @@ fn unionInit(
71287125 const union_type = try typeExpr(gz, scope, params[0]);
71297126 const field_name = try comptimeExpr(gz, scope, .{ .ty = .const_slice_u8_type }, params[1]);
71307127 switch (rl) {
7131 .none, .none_or_ref, .discard, .ref, .ty, .coerced_ty, .inferred_ptr => {
7128 .none, .discard, .ref, .ty, .coerced_ty, .inferred_ptr => {
71327129 _ = try gz.addPlNode(.field_type_ref, params[1], Zir.Inst.FieldTypeRef{
71337130 .container_type = union_type,
71347131 .field_name = field_name,
......@@ -7192,7 +7189,7 @@ fn bitCast(
71927189 const astgen = gz.astgen;
71937190 const dest_type = try typeExpr(gz, scope, lhs);
71947191 switch (rl) {
7195 .none, .none_or_ref, .discard, .ty, .coerced_ty => {
7192 .none, .discard, .ty, .coerced_ty => {
71967193 const operand = try expr(gz, scope, .none, rhs);
71977194 const result = try gz.addPlNode(.bitcast, node, Zir.Inst.Bin{
71987195 .lhs = dest_type,
......@@ -8799,7 +8796,7 @@ fn rvalue(
87998796) InnerError!Zir.Inst.Ref {
88008797 if (gz.endsWithNoReturn()) return result;
88018798 switch (rl) {
8802 .none, .none_or_ref, .coerced_ty => return result,
8799 .none, .coerced_ty => return result,
88038800 .discard => {
88048801 // Emit a compile error for discarding error values.
88058802 _ = try gz.addUnNode(.ensure_result_non_error, result, src_node);
......@@ -9561,9 +9558,7 @@ const GenZir = struct {
95619558 gz.rl_ty_inst = ty_inst;
95629559 gz.break_result_loc = parent_rl;
95639560 },
9564 .none_or_ref => {
9565 gz.break_result_loc = .ref;
9566 },
9561
95679562 .discard, .none, .ptr, .ref => {
95689563 gz.break_result_loc = parent_rl;
95699564 },