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

proposal: Go 2: allow trailing commas in the switch case enumerations #34088

Closed
ainar-g opened this issue Sep 4, 2019 · 11 comments
Closed

proposal: Go 2: allow trailing commas in the switch case enumerations #34088

ainar-g opened this issue Sep 4, 2019 · 11 comments
Labels
Milestone

Comments

@ainar-g
Copy link
Contributor

ainar-g commented Sep 4, 2019

Currently, multiline function calls allow (and require) trailing commas:

f(
	a,
	b,
	c,
)

Same with structs:

var v = T{
	a: a,
	b: b,
	c: c,
}

Same with arrays, maps, slices, etc. But when we try to use the same principles in a switch, we get an error:

switch unit {
case
	"ft",
	"km",
	"m",
	: // error: expected operand, found ':'
	return true 
default:
	return false
}

We have to leave a colon after the last element, which means that when a new unit appears below "m", the diffs will show changes in two lines instead of one:

-	"m":
+	"m",
+	"z":

Instead of:

+	"z",

I propose that the “comma-newline-colon” syntax becomes allowed.

@gopherbot gopherbot added this to the Proposal milestone Sep 4, 2019
@ainar-g ainar-g changed the title proposal: Go 2: allow trailing commas switch cases proposal: Go 2: allow trailing commas in the switch case enumerations Sep 4, 2019
@mvdan
Copy link
Member

mvdan commented Sep 4, 2019

I have to admit that while this could make the language more consistent, I think the comma-newline version of the switch case is weird. Maybe that's just because I'm not used to it.

On the other hand, do you have proof of developers or projects using this syntax, or having diff/conflict issues because of the syntax?

@ianlancetaylor ianlancetaylor added v2 A language change or incompatible library change LanguageChange labels Sep 4, 2019
@ainar-g
Copy link
Contributor Author

ainar-g commented Sep 5, 2019

This is a… peculiar syntax, but I couldn't think of a better one to achieve what is needed to make the consistent commas possible.

I'm not sure what you mean by “using this syntax”, since it's illegal now (or did you mean in other languages?). I've had issues in private code bases, and I'm not sure how to find proof of conflicts in open code bases. A quick grep found a lot of code that uses multiline cases, like in golang.org/x/tools, golang.org/x/crypto, and a quite big one—and thus most likely to change— in golang.org/x/net. I've also found this commit, which has exactly the kind of two-lines-more diff that I mentioned in the OP.

If you point me to a good way to find more conflicts, I'll gladly do that.

@beoran
Copy link

beoran commented Sep 10, 2019

This feature would be nice for making generating Go code easier. And I generate Go almost every day.

@griesemer
Copy link
Contributor

I disagree that this change would make the syntax more consistent. Currently, we allow a trailing comma in only two cases:

  • actual and formal parameter lists enclosed in parentheses
    (this includes conversions which show up separately in the spec but cannot be distinguished by the parser from function calls)
  • composite literal element lists enclosed in curly braces

They are essentially all a form of parameter lists.

In Go a comma serves as a "separator" of list elements, that is, it appears between elements (contrary to the semicolon which serves as a "terminator"). The only reason for permitting a trailing comma in the above mentioned exceptional cases is that in overlong parameter and composite literal element lists, it's not uncommon to write each element on a separate line, and the final closing parenthesis or curly brace on yet another new line. Allowing the comma permits easy extension of such lists.

In general elements of "lists" in Go (identifier lists, expression lists, and type lists) are separated by commas, not terminated by them. For instance, we can't allow a trailing comma after the c below as it would change the meaning of that declaration (and in this case make it invalid):

var a, b, c, int = 1, 2, 3,

In short, the trailing comma is an exception to ease an occasional annoyance; it was introduced very deliberately based on experience with syntactically related but more strict languages that didn't allow it (the Pascal line of languages).

Having separate switch and type switch cases on a separate line is much less common. As an aside, it's trivial for a code generator to get this right (after the last element, write ':' rather than ',').

So while I believe we could make the change for (type) switch cases and it would be backward-compatible, it would simply add another exception, and not make the language more consistent.

@ainar-g
Copy link
Contributor Author

ainar-g commented Sep 11, 2019

@griesemer Great comment, thanks! The difference between the lists in round or curly brackets and the lists without them hasn't occurred to me before. It makes sense. Your point about autogeneration also holds up.

In short, the trailing comma is an exception to ease an occasional annoyance; it was introduced very deliberately based on experience with syntactically related but more strict languages that didn't allow it (the Pascal line of languages).

I see. This proposal is essentially my “experience report” about a new “occasional annoyance”. Not a lot of mainstream languages allow multiple values in a case: the C-family languages don't allow it and have implicit fallthrough instead. Lua and Python don't have a switch-like statement at all.

Expression-based languages, like Perl and Ruby, allow something like this through their respective “smart matching” operators and arrays:

use v5.26;

use experimental ("switch");
use strict;

my $x = 42;

given ($x) {
when ([
    1,
    2,
    42,
]) {
    say("these");
}
default {
    say("other");
}
};
x = 42

case x
when
  *[
    1,
    2,
    42
  ]
  puts("these")
else
  puts("other")
end

Although in actual programmes those in-place anonymous arrays are mostly replaced with named ones. The Perl version is also experimental and may change in the future.

Rust allows something similar, although rustfmt wants to reformat it:

fn main() {
    let x = 42;

    match x {
        | 1
        | 2
        | 42
          => println!("these"),
        _ => println!("other"),
    };
}

It's also slightly ironic that you've mentioned Pascal. Thanks to the absence of the automatic semicolon insertion Pascal allows this:

PROGRAM Main;

VAR x: Integer;

BEGIN
	x := 42;

	CASE x OF
	1
	,
	2
	,
	42
	:
		WriteLn('these')
	ELSE
		WriteLn('other')
	END
END.

Which is not perfect and not exactly what this proposal is about but is still easier to edit (in Vim at least) and still produces nicer diffs.

The point that I'm trying to make with this comment is that multiline expression lists in case are mostly a new thing among popular non-expression-oriented languages without the match-a-scalar-with-an-array smartness. It is entirely possible that this is a thing that rarely bothers anyone but me. If that's the case, I'm fine with this proposal being rejected. But if after using the language (and the languages inspired by it) for a long time more people encounter the same issues then I think the proposal is at least worth considering.

@bradfitz
Copy link
Contributor

I also feel this pain pretty regularly (changing commas to colons and colons to commas), but the proposed syntax here with a colon on its own line feels ... weird.

I think it'd probably be better to live with the occasional annoyance, rather than have a weird syntax.

@griesemer
Copy link
Contributor

Given the feedback above, this is a likely-decline. Leaving open for 4 weeks for final comments/better syntax suggestions.

@ainar-g
Copy link
Contributor Author

ainar-g commented Oct 22, 2019

@bradfitz @griesemer An alternative syntax is allowing comma-separated lists in round brackets. Currently, this is legal:

switch x {
case
	(1),
	(2),
	(42):
	fmt.Println("these")
default:
	fmt.Println("those")
}

But this isn't:

switch x {
case (
	1,
	2,
	42,
):
	fmt.Println("these")
default:
	fmt.Println("those")
}

Unless I'm forgetting something, this could be achieved with the following changes to the Spec:

 ExprSwitchStmt = "switch" [ SimpleStmt ";" ] [ Expression ] "{" { ExprCaseClause } "}" .
 ExprCaseClause = ExprSwitchCase ":" StatementList .
-ExprSwitchCase = "case" ExpressionList | "default" .
+ExprSwitchCase = "case" ExprSwitchCaseList | "default" .
+ExprSwitchCaseList = ExpressionList | "(" ExpressionList [ "," ] ")" .
 TypeSwitchCase  = "case" TypeList | "default" .
 TypeSwitchStmt  = "switch" [ SimpleStmt ";" ] TypeSwitchGuard "{" { TypeCaseClause } "}" .
 TypeSwitchGuard = [ identifier ":=" ] PrimaryExpr "." "(" "type" ")" .
 TypeCaseClause  = TypeSwitchCase ":" StatementList .
-TypeSwitchCase  = "case" TypeList | "default" .
+TypeSwitchCase  = "case" TypeSwitchCaseList | "default" .
+TypeSwitchCaseList  = TypeList | "(" TypeList [ "," ] ")" .
 TypeList = Type { "," Type } .

Although that means that this:

switch x {
case (1):
	fmt.Println("these")
default:
	fmt.Println("those")
}

Could be interpreted both as an ExpressionList and as a one-element ExprSwitchCaseList. I'm not sure if this ambiguity is critical or not. If it is, another alternative is to use curly brackets:

switch x {
case {
	1,
	2,
	42,
}:
	fmt.Println("these")
default:
	fmt.Println("those")
}

From what I can see, this should not be ambiguous.

@griesemer
Copy link
Contributor

@ainar-g These syntax suggestion seem to make it worse - now I have to write some form of parentheses around lists? Just so that I can add an optional comma at the end? This cure seems worse than the disease.

@ainar-g
Copy link
Contributor Author

ainar-g commented Oct 22, 2019

@griesemer You don't have to, but you can opt-in to writing them, if you want to have trailing commas. I'm still trying to make this change backward-compatible after all. I personally think that the parentheses improve readability in this case, but I guess it depends on individual preferences.

@ianlancetaylor
Copy link
Contributor

No further comments on original proposal.

@golang golang locked and limited conversation to collaborators Nov 18, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

7 participants