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 fallthrough in type switches #40462

Closed
carnott-snap opened this issue Jul 28, 2020 · 10 comments
Closed

proposal: Go 2: allow fallthrough in type switches #40462

carnott-snap opened this issue Jul 28, 2020 · 10 comments

Comments

@carnott-snap
Copy link

@carnott-snap carnott-snap commented Jul 28, 2020

description

Currently type switches disallow fallthrough. For types that are convertible or cases that need pre-processing, it is useful to fallthrough. I propose allowing this for the unassigned format, since this should just work, and adding a new syntax for the assignment type.

switch v.(type) {
case bool:
        // ...
        fallthrough
case string:
        // ...
}

switch t := v.(type) {
case bool:
        fallthrough strconv.FormatBool(t)
case string:
        // ...
}

costs

This will add complexity to the parser, both in interpreting switch statements and having fallthrough accept a parameter. However it makes user's mental model for switch statements more homogenous, since now all can fallthrough, so it may be a win overall.

alternatives

If the parser changes are too complex, or having fallthrough accept a parameter is undesirable, we can use a custom syntax.

switch t := v.(type) {
case bool:
        t := strconv.FormatBool(t)
        fallthrough
case string:
        // ...
}
@gopherbot gopherbot added this to the Proposal milestone Jul 28, 2020
@gopherbot gopherbot added the Proposal label Jul 28, 2020
@martisch
Copy link
Contributor

@martisch martisch commented Jul 29, 2020

What is the type of variable t in the second switch example in the string case when falling through? There seems to be some semantics involved that are not explained in the proposal that should be stated explicitly.

Can you give a real world use case where this makes code more understandable?

@carnott-snap
Copy link
Author

@carnott-snap carnott-snap commented Jul 29, 2020

I list the unassigned variant for consistency, and because it seems trivial to allow. That being said, I rarely use it and agree the usefulness of that form is limited.

fallthrough would accept one parameter of a type matching the case statement below. I would think that the normal return coercion rules could be applied for consistency: type -> interface, etc.

This came up when amending a function that accepted interface{} and it was desired to support taking *pkg.Data in addition to pkg.Data, thus I wanted to:

switch t := v.(type) {
case pkg.Data:
        fallthrough &t
case *pkg.Data:
        // ...
}

That being said, this example can be extended to any case with two types that are interchangeable:

switch t := v.(type) {
case []byte:
        var d pkg.Data
        json.Unmarshal(t, &d)
        fallthrough d
case pkg.Data:
        // ...
}

I am also fairly certain that could be implemented without making a breaking change to the language.

@martisch
Copy link
Contributor

@martisch martisch commented Jul 29, 2020

So fallthrough accepts any expression as long as its assignable to the type of t in the next case block and the next block starts with t assigned the evaluation of the expression?

My mental model of switches gets more complicated because there is a keyword that changes the value of t without explicitly mentioning t. The alternative syntax is more complicated to me because the block scoping rules for variable declarations somehow work differently.

Note that from the example given above to me it would be written more readable as:

var d pkg.Data
switch t := v.(type) {
   case []byte:
     json.Unmarshal(t, &d)
   case pkg.Data:
     d = t
}
// ...
@carnott-snap
Copy link
Author

@carnott-snap carnott-snap commented Jul 29, 2020

So fallthrough accepts any expression as long as its assignable to the type of t in the next case block and the next block starts with t assigned the evaluation of the expression?

While I will not claim this as the only option, that works with my mental model and intuition.

My mental model of switches gets more complicated because there is a keyword that changes the value of t without explicitly mentioning t. The alternative syntax is more complicated to me because the block scoping rules for variable declarations somehow work differently.

While I follow your interpretation, it is pretty unambiguous what is happening, and you can still reach each case statement as an island. case pkg.Data: finishes and passes a *pkg.Data to the next case, its name is irrelevant to fallthrough. Then you can read the next case as isolated, since the compiler guarantees that fallthrough xxx meets the type for the next case.

Alternatively, do you have a better syntax for how to modify/set t? The only other though I had was to redo the assignment inline, preventing the scoping confusion:

switch t := v.(type) {
case pkg.Data:
        fallthrough t := &t // not sure if we should = or := though
case *pkg.Data:
        // ...
}

Note that from the example given above to me it would be written more readable as:

Sure, but this is tightly coupled with my example, and fails to scale to larger sets of cases:

switch t := v.(type) {
case pkg.One:
        fallthrough &t
case *pkg.One:
        // ...
case pkg.Two:
        fallthrough &t
case *pkg.Two:
        // ...
case []byte:
        var d pkg.Data
        json.Unmarshal(t, &d)
        fallthrough d
case pkg.Data:
        // ...
}
@martisch
Copy link
Contributor

@martisch martisch commented Jul 29, 2020

large switch cases already impact readability to me with or without fallthrough. the new example is better dealt with in my mind with (function names to be adjusted to reflect actual use better):

switch t := v.(type) {
case pkg.One:
        processPkgOne(&t)
case *pkg.One:
        processPkgOne(t)
case pkg.Two:
        processPkgTwo(&t)
case *pkg.Two:
        processPkgTwo(t)        
case []byte:
        var d pkg.Data
        json.Unmarshal(t, &d)
        processPkgData(d)
case pkg.Data:
        processPkgData(t)
}

Same number of lines on the screen for the swtich statement and I do not have to deal with the concept of fallthrough changing into the next block. Each block can be changed and removed without impacting any other block.

If the statements in each block are short and trivial (e.g. 1-3 lines) some lines can just be duplicated (as we save the fallthrough line) without taking up much more space. If the functionality is complex and common likely using a function makes sense for better testability. Has the added advantage that it can be used for any block in the switch case.

Another question would be why if *pkg.One and pkg.One behave the same can this is not be abstracted in an interface (which may also apply to pkg.Two) and all the cases together can be dealt with one interface conversion.

@ianlancetaylor ianlancetaylor changed the title proposal: allow fallthrough in type switches proposal: Go 2: allow fallthrough in type switches Jul 30, 2020
@carnott-snap
Copy link
Author

@carnott-snap carnott-snap commented Aug 6, 2020

@ianlancetaylor: can you add this to the proposals project?

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Aug 6, 2020

@carnott-snap Languages changes go through a separate process that doesn't use the proposals project.

@carnott-snap
Copy link
Author

@carnott-snap carnott-snap commented Aug 6, 2020

My apologies, I conflated the two.

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Aug 11, 2020

Per discussion above, this is a likely decline. Leaving open for four weeks for final comments

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Sep 15, 2020

No further discussion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.