Could/should this be a compiler warning?

Brainstorming
1

Just noticed I was doing something like this in a toString-like method on an enum in my app. It doesn’t really hurt anything, but it’s weird looking, and seems hoinestly like it should be invalid syntax, or at least warning-worthy. Sorry if this has been discussed before, tried searching the forum but came up empty.

const std = @import("std");

const Thing = enum {a, b};

fn doStuff(the: Thing) []const u8 {
	return switch (the) {
		.a => return "First value",
		.b => return "Second value",
	};
}

pub fn main() void {
	std.debug.print("{s}\n", .{doStuff(.a)});
}
1 Like
2

What’s the problem?

1 Like
3

It doesn’t really hurt anything, but yes, it is kind of unidiomatic to have switch prongs return when the switch itself is an expression to a return.

Allowing switch prongs to contain returns does have valid use cases, though, for “short circuiting” a function in certain cases. E.g. you couldd have something like this:

const Thing = union(enum) { a: u32, b };

fn doStuff(the: Thing) []const u8 {
	const v = switch (the) {
		.a => |a_value| a_value,
		.b => return "Second value", // here, the function ends immediately
	};

	// do something more complex here with V

	return some result;
}

Side note: Zig doesn’t have warnings and the goal as far as I understand is for it to never have any. If this case ever gets treated, it’ll probably become an error.

2 Likes
4

An unreachable “return” (the one returning the result of the switch statement, of which both branches return on their own instead of producing a value.)

5

why is this a problem? it looks like that return statement is appropriately noreturn.

2 Likes
6

I assume that the OP expects any unreachable code to produce a warning, which is a reasonable assumption IMO considering a lot of newer languages do that

4 Likes
7

It’s not a problem per se. I guess it just looks weird. The “correct” way to write it would either be

fn doStuff(the: Thing) []const u8 {
	switch (the) {
		.a => return "First value",
		.b => return "Second value",
	};
}

or even better

fn doStuff(the: Thing) []const u8 {
	return switch (the) {
		.a => "First value",
		.b => "Second value",
	};
}

In the original case, either the first return or the other two are basically redundant.

EDIT: At least that’s how I understand OP’s question.

3 Likes
8

Yeah, this exactly.

9

I guess this is because of how the lazy analysis works. When all the switch prongs return early, the compiler doesn’t even look at the rest of the function. This also compiles without error:

fn doStuff(the: Thing) []const u8 {
    const what = switch (the) {
        .a => return "First value",
        .b => return "Second value",
    };
    const nothing = what + 42;
    nothing.nowhere(what);
    return 3.1415 and "nope";
}
6 Likes
10

Side-side note: there is always the possibility of a comptime causing unreachable code, e.g. on some platform, early return an error because it’s not supported. Wouldn’t make sense on that platform to annotate that “yes, this early return is meant to return early”. Analyzing more than the current comptime would be wasteful during compilation.

Maybe it would be in the domain of zls to produce that kind of warning, i.e. to consider many comptime possibilities and do some analysis on that.

5 Likes
11

I think a good comparison is how

unreachable;
return 5;

is a compile error but

if (true) unreachable;
return 5;

is not. I think that although this particular example looks weird, this is correct behavior.

4 Likes
12

Fair enough, this made the logic click in my head. Thanks!

1 Like