Skip to content

Add error message for unreachable else prong in switch #5693

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
Jul 26, 2020
Merged

Add error message for unreachable else prong in switch #5693

merged 3 commits into from
Jul 26, 2020

Conversation

antlilja
Copy link
Contributor

@antlilja antlilja commented Jun 24, 2020

Closes #5623.

All the compile error tests are passing, but the check for error set unreachable else prongs are causing some problems.
PR no longer contains checks for error sets

The else prong might be required in one context and not in another. This is probably most apparent in generics where the error set could depend on some argument.

Example in std/io/reader.zig:

pub fn Reader(
    comptime Context: type,
    comptime ReadError: type,
    /// Returns the number of bytes read. It may be less than buffer.len.
    /// If the number of bytes read is 0, it means end of stream.
    /// End of stream is not an error condition.
    comptime readFn: fn (context: Context, buffer: []u8) ReadError!usize,
) type {
    return struct {
        pub const Error = ReadError;

        context: Context,

        const Self = @This();

        /// Returns the number of bytes read. It may be less than buffer.len.
        /// If the number of bytes read is 0, it means end of stream.
        /// End of stream is not an error condition.
        pub fn read(self: Self, buffer: []u8) Error!usize {
            return readFn(self.context, buffer);
        }
        ...

        pub fn readUntilDelimiterOrEof(self: Self, buf: []u8, delimiter: u8) !?[]u8 {
            var index: usize = 0;
            while (true) {
                const byte = self.readByte() catch |err| switch (err) {
                    error.EndOfStream => {
                        if (index == 0) {
                            return null;
                        } else {
                            return buf[0..index];
                        }
                    },
                    else => |e| return e, // This is sometimes an error depending on if ReadError contains any more errors
                };

                if (byte == delimiter) return buf[0..index];
                if (index >= buf.len) return error.StreamTooLong;

                buf[index] = byte;
                index += 1;
            }
        }
        ...

        pub fn readByte(self: Self) !u8 {
            var result: [1]u8 = undefined;
            const amt_read = try self.read(result[0..]); // an error could be returned from here but not always
            if (amt_read < 1) return error.EndOfStream; // this error can always be returned
            return result[0];
        }
        ...

This might example might generate error: unreachable else prong depending on if ReadError contains any more errors.

We could maybe get a similar effect from compile time constants such as target?

This PR could be merged if we remove the error message for error sets.

@antlilja antlilja marked this pull request as draft June 24, 2020 13:44
antlilja added 2 commits June 24, 2020 17:36
* Remove have_else_prong (bool)
* Add else_prong (AstNode*)
@antlilja antlilja marked this pull request as ready for review June 24, 2020 17:14
@antlilja
Copy link
Contributor Author

I've removed the unreachable else prong check on error sets so this PR is now mergeable with only checks for bools, ints and enums.

@antlilja antlilja changed the title Add error message for unreachable else prong in switch #5623 Add error message for unreachable else prong in switch Jun 24, 2020
* Adds error message for types: enum, int and bool
* Adds compile error tests
@andrewrk andrewrk merged commit a36772e into ziglang:master Jul 26, 2020
@andrewrk
Copy link
Member

Nice work. That's a good question about error sets. It's a difficult design problem, related to lazy decl analysis and #3028.

@antlilja antlilja deleted the switch-unreachable-else branch July 26, 2020 21:38
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.

Disallow unreachable "else" in switch
2 participants