Merge pull request #1109 from ziglang/pass-by-non-copying-value

allow passing by non-copying value
ziglang · Jun 17, 2018 · 7515187 · 7515187
2 parents 3ee4d23 + 472b7ef
commit 7515187
Show file tree

Hide file tree

Showing 16 changed files with 306 additions and 344 deletions.
diff --git a/doc/langref.html.in b/doc/langref.html.in
@@ -2818,39 +2818,30 @@ fn foo() void { }
       {#code_end#}
       {#header_open|Pass-by-value Parameters#}
       <p>
-      In Zig, structs, unions, and enums with payloads cannot be passed by value
-      to a function.
+      In Zig, structs, unions, and enums with payloads can be passed directly to a function:
       </p>
-      {#code_begin|test_err|not copyable; cannot pass by value#}
-const Foo = struct {
+      {#code_begin|test#}
+const Point = struct {
     x: i32,
+    y: i32,
 };
 
-fn bar(foo: Foo) void {}
-
-test "pass aggregate type by value to function" {
-    bar(Foo {.x = 12,});
+fn foo(point: Point) i32 {
+    return point.x + point.y;
 }
-      {#code_end#}
-      <p>
-      Instead, one must use <code>*const</code>. Zig allows implicitly casting something
-      to a const pointer to it:
-      </p>
-      {#code_begin|test#}
-const Foo = struct {
-    x: i32,
-};
 
-fn bar(foo: *const Foo) void {}
+const assert = @import("std").debug.assert;
 
-test "implicitly cast to const pointer" {
-    bar(Foo {.x = 12,});
+test "pass aggregate type by non-copy value to function" {
+    assert(foo(Point{ .x = 1, .y = 2 }) == 3);
 }
       {#code_end#}
       <p>
-      However,
-      the C ABI does allow passing structs and unions by value. So functions which
-      use the C calling convention may pass structs and unions by value.
+      In this case, the value may be passed by reference, or by value, whichever way
+      Zig decides will be faster.
+      </p>
+      <p>
+      For extern functions, Zig follows the C ABI for passing structs and unions by value.
       </p>
       {#header_close#}
       {#header_open|Function Reflection#}

diff --git a/src/analyze.cpp b/src/analyze.cpp
@@ -1135,7 +1135,10 @@ TypeTableEntry *get_fn_type(CodeGen *g, FnTypeId *fn_type_id) {
             gen_param_info->src_index = i;
             gen_param_info->gen_index = SIZE_MAX;
 
-            type_ensure_zero_bits_known(g, type_entry);
+            ensure_complete_type(g, type_entry);
+            if (type_is_invalid(type_entry))
+                return g->builtin_types.entry_invalid;
+
             if (type_has_bits(type_entry)) {
                 TypeTableEntry *gen_type;
                 if (handle_is_ptr(type_entry)) {
@@ -1546,12 +1549,6 @@ static TypeTableEntry *analyze_fn_type(CodeGen *g, AstNode *proto_node, Scope *c
             case TypeTableEntryIdUnion:
             case TypeTableEntryIdFn:
             case TypeTableEntryIdPromise:
-                ensure_complete_type(g, type_entry);
-                if (calling_convention_allows_zig_types(fn_type_id.cc) && !type_is_copyable(g, type_entry)) {
-                    add_node_error(g, param_node->data.param_decl.type,
-                        buf_sprintf("type '%s' is not copyable; cannot pass by value", buf_ptr(&type_entry->name)));
-                    return g->builtin_types.entry_invalid;
-                }
                 break;
         }
         FnTypeParamInfo *param_info = &fn_type_id.param_info[fn_type_id.next_param_index];

diff --git a/src/codegen.cpp b/src/codegen.cpp
@@ -326,13 +326,6 @@ static void addLLVMArgAttr(LLVMValueRef arg_val, unsigned param_index, const cha
     return addLLVMAttr(arg_val, param_index + 1, attr_name);
 }
 
-static void addLLVMCallsiteAttr(LLVMValueRef call_instr, unsigned param_index, const char *attr_name) {
-    unsigned kind_id = LLVMGetEnumAttributeKindForName(attr_name, strlen(attr_name));
-    assert(kind_id != 0);
-    LLVMAttributeRef llvm_attr = LLVMCreateEnumAttribute(LLVMGetGlobalContext(), kind_id, 0);
-    LLVMAddCallSiteAttribute(call_instr, param_index + 1, llvm_attr);
-}
-
 static bool is_symbol_available(CodeGen *g, Buf *name) {
     return g->exported_symbol_names.maybe_get(name) == nullptr && g->external_prototypes.maybe_get(name) == nullptr;
 }
@@ -581,11 +574,6 @@ static LLVMValueRef fn_llvm_value(CodeGen *g, FnTableEntry *fn_table_entry) {
         if (param_type->id == TypeTableEntryIdPointer) {
             addLLVMArgAttr(fn_table_entry->llvm_value, (unsigned)gen_index, "nonnull");
         }
-        // Note: byval is disabled on windows due to an LLVM bug:
-        // https://github.com/ziglang/zig/issues/536
-        if (is_byval && g->zig_target.os != OsWindows) {
-            addLLVMArgAttr(fn_table_entry->llvm_value, (unsigned)gen_index, "byval");
-        }
     }
 
     uint32_t err_ret_trace_arg_index = get_err_ret_trace_arg_index(g, fn_table_entry);
@@ -3114,15 +3102,6 @@ static LLVMValueRef ir_render_call(CodeGen *g, IrExecutable *executable, IrInstr
     }
 
 
-    for (size_t param_i = 0; param_i < fn_type_id->param_count; param_i += 1) {
-        FnGenParamInfo *gen_info = &fn_type->data.fn.gen_param_info[param_i];
-        // Note: byval is disabled on windows due to an LLVM bug:
-        // https://github.com/ziglang/zig/issues/536
-        if (gen_info->is_byval && g->zig_target.os != OsWindows) {
-            addLLVMCallsiteAttr(result, (unsigned)gen_info->gen_index, "byval");
-        }
-    }
-
     if (instruction->is_async) {
         LLVMValueRef payload_ptr = LLVMBuildStructGEP(g->builder, instruction->tmp_ptr, err_union_payload_index, "");
         LLVMBuildStore(g->builder, result, payload_ptr);

diff --git a/src/ir.cpp b/src/ir.cpp
@@ -10463,13 +10463,6 @@ static IrInstruction *ir_implicit_cast(IrAnalyze *ira, IrInstruction *value, Typ
     zig_unreachable();
 }
 
-static IrInstruction *ir_implicit_byval_const_ref_cast(IrAnalyze *ira, IrInstruction *inst) {
-    if (type_is_copyable(ira->codegen, inst->value.type))
-        return inst;
-    TypeTableEntry *const_ref_type = get_pointer_to_type(ira->codegen, inst->value.type, true);
-    return ir_implicit_cast(ira, inst, const_ref_type);
-}
-
 static IrInstruction *ir_get_deref(IrAnalyze *ira, IrInstruction *source_instruction, IrInstruction *ptr) {
     TypeTableEntry *type_entry = ptr->value.type;
     if (type_is_invalid(type_entry)) {
@@ -12283,7 +12276,7 @@ static bool ir_analyze_fn_call_generic_arg(IrAnalyze *ira, AstNode *fn_proto_nod
     IrInstruction *casted_arg;
     if (is_var_args) {
         arg_part_of_generic_id = true;
-        casted_arg = ir_implicit_byval_const_ref_cast(ira, arg);
+        casted_arg = arg;
     } else {
         if (param_decl_node->data.param_decl.var_token == nullptr) {
             AstNode *param_type_node = param_decl_node->data.param_decl.type;
@@ -12296,7 +12289,7 @@ static bool ir_analyze_fn_call_generic_arg(IrAnalyze *ira, AstNode *fn_proto_nod
                 return false;
         } else {
             arg_part_of_generic_id = true;
-            casted_arg = ir_implicit_byval_const_ref_cast(ira, arg);
+            casted_arg = arg;
         }
     }
 
@@ -12515,9 +12508,18 @@ static TypeTableEntry *ir_analyze_fn_call(IrAnalyze *ira, IrInstructionCall *cal
 
         size_t next_proto_i = 0;
         if (first_arg_ptr) {
-            IrInstruction *first_arg;
             assert(first_arg_ptr->value.type->id == TypeTableEntryIdPointer);
-            if (handle_is_ptr(first_arg_ptr->value.type->data.pointer.child_type)) {
+
+            bool first_arg_known_bare = false;
+            if (fn_type_id->next_param_index >= 1) {
+                TypeTableEntry *param_type = fn_type_id->param_info[next_proto_i].type;
+                if (type_is_invalid(param_type))
+                    return ira->codegen->builtin_types.entry_invalid;
+                first_arg_known_bare = param_type->id != TypeTableEntryIdPointer;
+            }
+
+            IrInstruction *first_arg;
+            if (!first_arg_known_bare && handle_is_ptr(first_arg_ptr->value.type->data.pointer.child_type)) {
                 first_arg = first_arg_ptr;
             } else {
                 first_arg = ir_get_deref(ira, first_arg_ptr, first_arg_ptr);
@@ -12667,9 +12669,18 @@ static TypeTableEntry *ir_analyze_fn_call(IrAnalyze *ira, IrInstructionCall *cal
         size_t next_proto_i = 0;
 
         if (first_arg_ptr) {
-            IrInstruction *first_arg;
             assert(first_arg_ptr->value.type->id == TypeTableEntryIdPointer);
-            if (handle_is_ptr(first_arg_ptr->value.type->data.pointer.child_type)) {
+
+            bool first_arg_known_bare = false;
+            if (fn_type_id->next_param_index >= 1) {
+                TypeTableEntry *param_type = fn_type_id->param_info[next_proto_i].type;
+                if (type_is_invalid(param_type))
+                    return ira->codegen->builtin_types.entry_invalid;
+                first_arg_known_bare = param_type->id != TypeTableEntryIdPointer;
+            }
+
+            IrInstruction *first_arg;
+            if (!first_arg_known_bare && handle_is_ptr(first_arg_ptr->value.type->data.pointer.child_type)) {
                 first_arg = first_arg_ptr;
             } else {
                 first_arg = ir_get_deref(ira, first_arg_ptr, first_arg_ptr);
@@ -12802,10 +12813,7 @@ static TypeTableEntry *ir_analyze_fn_call(IrAnalyze *ira, IrInstructionCall *cal
                     return ira->codegen->builtin_types.entry_invalid;
             }
             if (inst_fn_type_id.async_allocator_type == nullptr) {
-                IrInstruction *casted_inst = ir_implicit_byval_const_ref_cast(ira, uncasted_async_allocator_inst);
-                if (type_is_invalid(casted_inst->value.type))
-                    return ira->codegen->builtin_types.entry_invalid;
-                inst_fn_type_id.async_allocator_type = casted_inst->value.type;
+                inst_fn_type_id.async_allocator_type = uncasted_async_allocator_inst->value.type;
             }
             async_allocator_inst = ir_implicit_cast(ira, uncasted_async_allocator_inst, inst_fn_type_id.async_allocator_type);
             if (type_is_invalid(async_allocator_inst->value.type))
@@ -12866,20 +12874,23 @@ static TypeTableEntry *ir_analyze_fn_call(IrAnalyze *ira, IrInstructionCall *cal
     IrInstruction **casted_args = allocate<IrInstruction *>(call_param_count);
     size_t next_arg_index = 0;
     if (first_arg_ptr) {
-        IrInstruction *first_arg;
         assert(first_arg_ptr->value.type->id == TypeTableEntryIdPointer);
-        if (handle_is_ptr(first_arg_ptr->value.type->data.pointer.child_type)) {
+
+        TypeTableEntry *param_type = fn_type_id->param_info[next_arg_index].type;
+        if (type_is_invalid(param_type))
+            return ira->codegen->builtin_types.entry_invalid;
+
+        IrInstruction *first_arg;
+        if (param_type->id == TypeTableEntryIdPointer &&
+            handle_is_ptr(first_arg_ptr->value.type->data.pointer.child_type))
+        {
             first_arg = first_arg_ptr;
         } else {
             first_arg = ir_get_deref(ira, first_arg_ptr, first_arg_ptr);
             if (type_is_invalid(first_arg->value.type))
                 return ira->codegen->builtin_types.entry_invalid;
         }
 
-        TypeTableEntry *param_type = fn_type_id->param_info[next_arg_index].type;
-        if (type_is_invalid(param_type))
-            return ira->codegen->builtin_types.entry_invalid;
-
         IrInstruction *casted_arg = ir_implicit_cast(ira, first_arg, param_type);
         if (type_is_invalid(casted_arg->value.type))
             return ira->codegen->builtin_types.entry_invalid;

diff --git a/std/array_list.zig b/std/array_list.zig
@@ -29,36 +29,36 @@ pub fn AlignedArrayList(comptime T: type, comptime A: u29) type {
             };
         }
 
-        pub fn deinit(self: *const Self) void {
+        pub fn deinit(self: Self) void {
             self.allocator.free(self.items);
         }
 
-        pub fn toSlice(self: *const Self) []align(A) T {
+        pub fn toSlice(self: Self) []align(A) T {
             return self.items[0..self.len];
         }
 
-        pub fn toSliceConst(self: *const Self) []align(A) const T {
+        pub fn toSliceConst(self: Self) []align(A) const T {
             return self.items[0..self.len];
         }
 
-        pub fn at(self: *const Self, n: usize) T {
+        pub fn at(self: Self, n: usize) T {
             return self.toSliceConst()[n];
         }
 
         /// Sets the value at index `i`, or returns `error.OutOfBounds` if
         /// the index is not in range.
-        pub fn setOrError(self: *const Self, i: usize, item: *const T) !void {
+        pub fn setOrError(self: Self, i: usize, item: T) !void {
             if (i >= self.len) return error.OutOfBounds;
-            self.items[i] = item.*;
+            self.items[i] = item;
         }
 
         /// Sets the value at index `i`, asserting that the value is in range.
-        pub fn set(self: *const Self, i: usize, item: *const T) void {
+        pub fn set(self: *Self, i: usize, item: T) void {
             assert(i < self.len);
-            self.items[i] = item.*;
+            self.items[i] = item;
         }
 
-        pub fn count(self: *const Self) usize {
+        pub fn count(self: Self) usize {
             return self.len;
         }
 
@@ -81,12 +81,12 @@ pub fn AlignedArrayList(comptime T: type, comptime A: u29) type {
             return result;
         }
 
-        pub fn insert(self: *Self, n: usize, item: *const T) !void {
+        pub fn insert(self: *Self, n: usize, item: T) !void {
             try self.ensureCapacity(self.len + 1);
             self.len += 1;
 
             mem.copy(T, self.items[n + 1 .. self.len], self.items[n .. self.len - 1]);
-            self.items[n] = item.*;
+            self.items[n] = item;
         }
 
         pub fn insertSlice(self: *Self, n: usize, items: []align(A) const T) !void {
@@ -97,9 +97,9 @@ pub fn AlignedArrayList(comptime T: type, comptime A: u29) type {
             mem.copy(T, self.items[n .. n + items.len], items);
         }
 
-        pub fn append(self: *Self, item: *const T) !void {
+        pub fn append(self: *Self, item: T) !void {
             const new_item_ptr = try self.addOne();
-            new_item_ptr.* = item.*;
+            new_item_ptr.* = item;
         }
 
         pub fn appendSlice(self: *Self, items: []align(A) const T) !void {

diff --git a/std/build.zig b/std/build.zig
@@ -234,7 +234,7 @@ pub const Builder = struct {
         defer wanted_steps.deinit();
 
         if (step_names.len == 0) {
-            try wanted_steps.append(&self.default_step);
+            try wanted_steps.append(self.default_step);
         } else {
             for (step_names) |step_name| {
                 const s = try self.getTopLevelStepByName(step_name);

diff --git a/std/fmt/index.zig b/std/fmt/index.zig
@@ -162,8 +162,6 @@ pub fn formatType(
             },
             builtin.TypeInfo.Pointer.Size.Many => {
                 if (ptr_info.child == u8) {
-                    //This is a bit of a hack, but it made more sense to
-                    // do this check here than have formatText do it
                     if (fmt[0] == 's') {
                         const len = std.cstr.len(value);
                         return formatText(value[0..len], fmt, context, Errors, output);
@@ -176,6 +174,12 @@ pub fn formatType(
                 return output(context, casted_value);
             },
         },
+        builtin.TypeId.Array => |info| {
+            if (info.child == u8) {
+                return formatText(value, fmt, context, Errors, output);
+            }
+            return format(context, Errors, output, "{}@{x}", @typeName(T.Child), @ptrToInt(&value));
+        },
         else => @compileError("Unable to format type '" ++ @typeName(T) ++ "'"),
     }
 }

diff --git a/std/json.zig b/std/json.zig
@@ -1326,7 +1326,7 @@ pub const Parser = struct {
             },
             // Array Parent -> [ ..., <array>, value ]
             Value.Array => |*array| {
-                try array.append(value);
+                try array.append(value.*);
                 p.state = State.ArrayValue;
             },
             else => {