Skip to content
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

make aggregate types non-copyable by default; provide copyable attribute #3804

Closed
andrewrk opened this issue Nov 29, 2019 · 5 comments
Closed
Labels
proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Milestone

Comments

@andrewrk
Copy link
Member

andrewrk commented Nov 29, 2019

This is a competing proposal with #3803. It solves the same problem in a different way, which is arguably simpler, and safer by default.

It's pretty easy to explain: make all aggregate types non-copyable by default. So this would be an error:

const Point = struct {
    x: i32,
    y: i32,
};

test "copy a point" {
    var pt = Point{.x = 1, .y = 2};
    var pt2 = pt; // error: copy of struct which does not have the `copyok` attribute
}

But this would work:

const Point2 = struct copyok {
    x: i32,
    y: i32,
};

test "copy a point" {
    var pt = Point{.x = 1, .y = 2};
    var pt2 = pt; // OK
}

Some notes:

@andrewrk andrewrk added the proposal This issue suggests modifications. If it also has the "accepted" label then it is planned. label Nov 29, 2019
@andrewrk andrewrk added this to the 0.6.0 milestone Nov 29, 2019
@andrewrk andrewrk modified the milestones: 0.6.0, 0.7.0 Nov 30, 2019
@daurnimator
Copy link
Collaborator

daurnimator commented Nov 30, 2019

* Enums, anonymous struct literals and anonymous list literals would be copyable

I don't think enums should be, what about something like:

const Foo = struct {
    const Self = @This();

    x: i32,
    qux: enum {
       a, b, c,
       pub fn validate(thisenum: *@This()) bool {
           const self = @fieldParentPtr(Self, "qux", thisenum);
           return switch (thisenum) {
               a => self.x < 42,
               b => self.x < 100,
               c => self.x >= 50,
           };
        }
    },
};

@ikskuh
Copy link
Contributor

ikskuh commented Nov 30, 2019

As good as it sounds, this proposal implies explicit move- and copy-semantics for a lot of types and special care for a lot of generics:

zig/lib/std/array_list.zig

Lines 139 to 142 in 85e1e3b

pub fn append(self: *Self, item: T) !void {
const new_item_ptr = try self.addOne();
new_item_ptr.* = item;
}

This would not work with non-copyok types, so most of std containers would have to be implemented again with special care for non-copyable types.

And as @daurnimator said: @fieldParentPtr gives us freedom to use any type for "inheritance", so any type should be markable as non-copyable

@emekoi
Copy link
Contributor

emekoi commented Dec 1, 2019

isn't this basically the same thing as #3803 but with pinned types rather than fields? perhaps these can be unified by having a @Pin attribute that marks types as immovable.
this proposal becomes:

const Point2 = @Pin(struct {
    x: i32,
    y: i32,
});

and #3803 becomes:

const FixedBufferAllocator = struct {
    allocator: @Pin(Allocator),
    buf: []u8,
};

in cases that it doesn't make sense for the type to be movable such as function pointers @Pin would be a compiler error.

@frmdstryr
Copy link
Contributor

Perhaps there should be a special copy operator := for aggregate types to make the intent clear?

@SpexGuy
Copy link
Contributor

SpexGuy commented Jan 13, 2021

Closing this in favor of #7769. As that issue points out, copy prevention is only tangentially related to RAII, and doesn't really do a good job of replacing it. Once those use cases are removed, it seems silly to disallow copies on most structs.

@SpexGuy SpexGuy closed this as completed Jan 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Projects
None yet
Development

No branches or pull requests

6 participants