Skip to content

allow passing by non-copying value #1109

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jun 17, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 14 additions & 23 deletions doc/langref.html.in
Original file line number Diff line number Diff line change
Expand Up @@ -2797,39 +2797,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#}
Expand Down
11 changes: 4 additions & 7 deletions src/analyze.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand Down Expand Up @@ -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];
Expand Down
21 changes: 0 additions & 21 deletions src/codegen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
57 changes: 34 additions & 23 deletions src/ir.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand Down Expand Up @@ -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;
Expand All @@ -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;
}
}

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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;
Expand Down
26 changes: 13 additions & 13 deletions std/array_list.zig
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand All @@ -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 {
Expand All @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion std/build.zig
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
8 changes: 6 additions & 2 deletions std/fmt/index.zig
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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) ++ "'"),
}
}
Expand Down
2 changes: 1 addition & 1 deletion std/json.zig
Original file line number Diff line number Diff line change
Expand Up @@ -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 => {
Expand Down
Loading