Skip to content

Add SliceType and ArrayPointerType to std.meta #2782

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

Closed
wants to merge 1 commit into from

Conversation

marler8997
Copy link
Contributor

@marler8997 marler8997 commented Jun 28, 2019

Provide a couple functions in std.meta to transform pointer/array types to the slice/array pointer variations. Note that the key feature here is that it preserves all pointer attributes (i.e. const, volatile, etc)

    testing.expect(SliceType([*]u8) == []u8);
    testing.expect(SliceType([*]const u8) == []const u8);

    testing.expect(ArrayPointerType([]u8) == [*]u8);
    testing.expect(ArrayPointerType([]const u8) == [*]const u8);

I've converted 3 projects to zig and have already made use of these functions in all 3 projects.

@daurnimator daurnimator added the standard library This issue involves writing Zig code for the standard library. label Aug 2, 2019
@andrewrk andrewrk added this to the 0.5.0 milestone Aug 19, 2019
@andrewrk
Copy link
Member

andrewrk commented Sep 4, 2019

This should be unblocked now.

@marler8997 marler8997 force-pushed the metaArrayHelpers branch 2 times, most recently from b3fd6ea to efd714d Compare September 4, 2019 16:56
std/meta.zig Outdated
}

test "std.meta.SliceType" {
testing.expect(SliceType([]u8) == []u8);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure how much testing I should do here...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Basic functionality is fine.

std/meta.zig Outdated
@@ -115,6 +115,66 @@ test "std.meta.Child" {
testing.expect(Child(?u8) == u8);
}

/// Given an array/pointer type, return the slice type `[]Child`.
/// Preserves all pointer attributes such as `const`/`volatile` etc.
pub fn SliceType(comptime T: type) type {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is Type redundant? Should it just be Slice and ArrayPointer rather than SliceType and ArrayPointerType?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer without the Type suffix since we have a pretty consistent formatting style that types are capitalized.

std/meta.zig Outdated
testing.expect(Slice(*const u8) == []const u8);
testing.expect(Slice([*]u8) == []u8);
testing.expect(Slice([*]const u8) == []const u8);
//testing.expect(Slice([10]u8) == []const u8);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's have this either be a real test case or deleted. It's not clear what this comment is communicating here.

How about this test case?
testing.expect(Slice(*[10]u8) == []const u8);

Same comment for the other commented out test case below.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking we would integrate #3173 first, then uncomment this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's a smell that the function would accept a non-reference type and return a reference type. I think the test case I provided is the one you're looking for here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh sorry I misunderstood your point. Yes I think *[N]T makes more sense than [N]T. Will change

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the question is, should Slice(*[N]T) become [][N]T or []T?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Slice(*[N]T) should become []T to be consistent with how type coercion works in the rest of the language.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm, given that answer, I'm not sure what the rules are. It sounds like we have one or more special cases for single-value pointers? What would these cases be?

if (single_value_pointer)
    if (pointer_to_array) return []T.child.child
    else return []T.child

Are there any other cases where we would return []T.child.child?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there any other cases where we would return []T.child.child?

Single item pointer to array is the only case that matches type coercion semantics

@andrewrk andrewrk modified the milestones: 0.5.0, 0.6.0 Sep 4, 2019
@marler8997 marler8997 mentioned this pull request Sep 4, 2019
@andrewrk andrewrk removed this from the 0.6.0 milestone Oct 1, 2019
@andrewrk andrewrk added the work in progress This pull request is not ready for review yet. label Nov 26, 2019
@andrewrk
Copy link
Member

Feel free to re-open when this is ready for review

@andrewrk andrewrk closed this Dec 10, 2019
@marler8997
Copy link
Contributor Author

@andrewrk I had an outstanding question I was waiting for you to answer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
standard library This issue involves writing Zig code for the standard library. work in progress This pull request is not ready for review yet.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants