Skip to content

Improve 'slice of single-item pointer' error message - #14673

Closed
hazeycode wants to merge 4 commits into
ziglang:masterfrom
hazeycode:master
Closed

Improve 'slice of single-item pointer' error message#14673
hazeycode wants to merge 4 commits into
ziglang:masterfrom
hazeycode:master

Conversation

@hazeycode

@hazeycode hazeycode commented Feb 18, 2023

Copy link
Copy Markdown
Contributor

Offer a correct explicit coercion syntax to help the user avoid other footgunnable workarounds when they encounter this error.

Arguably, there's no need for this if we allowed explicit coercion using slice syntax [0..] or similar (also see #8197 and #3156) but this provides immediate value to users today per the status quo.

@hazeycode hazeycode changed the title Improve 'slice of single-item pointer' error message Draft: Improve 'slice of single-item pointer' error message Feb 18, 2023
@hazeycode
hazeycode marked this pull request as draft February 18, 2023 14:32
@hazeycode

hazeycode commented Feb 19, 2023

Copy link
Copy Markdown
Contributor Author

Fixed up to only emit note when the user has typed foo[0..] or foo[0..1], otherwise behaves as before. Thanks to @InKryption for the help with this!

@hazeycode
hazeycode marked this pull request as ready for review February 19, 2023 02:31
@hazeycode hazeycode changed the title Draft: Improve 'slice of single-item pointer' error message Improve 'slice of single-item pointer' error message Feb 19, 2023
Comment thread src/Sema.zig Outdated

@Vexu Vexu left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Needs a test.

Comment thread src/Sema.zig Outdated
const start_val = try sema.resolveDefinedValue(block, start_src, uncasted_start) orelse {
break :emit_note false;
};
const start = start_val.toUnsignedInt(sema.mod.getTarget());

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The types of the the start and end values have not yet been validated at this point so it is not safe to use toUnsignedInt.

@hazeycode hazeycode May 15, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks. What is the proper way to validate and handle invalid values here?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Calling getUnsignedIntAdvanced will give you an optional int that will be null if it can't fit in a u64 which should be enough for this, just check that it's not undefined first.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks! I've made changes. Have I understood correctly?

@hazeycode
hazeycode requested a review from Vexu May 15, 2023 22:29

@andrewrk andrewrk left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Inspired by this PR, I decided to make the language allow slicing a single-item pointer from [0..1] if the start and end are comptime-known: #16075

I believe this will make the compile error note much more straightforward to implement, and give better advice.

Let us move this compile error enhancement effort into the implementation for that proposal.

// target=native
//
// :2:22: error: slice of single-item pointer
// :2:22: note: single-item pointer can be coerced to array using '@as(*[1]i32, ptr)'

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

*[1]i32 is not an array, it is a pointer to an array. Compile errors must be pedantically precise.

@andrewrk andrewrk closed this Jun 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants