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: short function declarations with named function type #60424

Closed
chewxy opened this issue May 25, 2023 · 11 comments
Closed

proposal: Go 2: short function declarations with named function type #60424

chewxy opened this issue May 25, 2023 · 11 comments
Labels
LanguageChange Proposal v2 A language change or incompatible library change WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Milestone

Comments

@chewxy
Copy link

chewxy commented May 25, 2023

Consider the following:

type Foo func(a int) int

func makeFoo1(...) Foo {
    return func(a int) int { ... }
}

func makeFoo2(...) Foo {
    return func(a int) int { ... }
}

This is for the most part fine, if the function signature is short. However, when it comes to longer function signatures for Foo, then there's lots to type (on the keyboard).

Proposal

Allow literal definition of functions of a named function type, if the parameter variables are already bound in the definition of the type:

type Foo func(a, b, c, d, e, f, g, h, i int, j, k, l, m string) bool 

func makeFoo1(...) Foo {
    return Foo { 
        fmt.Printf("%d\n", g) // uses `g`, which is pre-bound to the 7th argument by the type definition above
    }
}

So what happens if Foo doesn't have variables bound to the parameters?

type Foo func(int, int) int 

func makeFoo1(...) Foo {
    return Foo {...}
}

I have two suggestions:

  1. It fails to compile, because the named function type doesn't define a default variable name for each of the parameters.
  2. Allow late(r) binding (but still within the compile phase) of variable names to the parameters... something like this:
type Foo func(int, int) int

func makeFoo1(...) Foo {
    return Foo(a, b int) int { ... }
}

Currently

What the parser currently does is that it treats Foo{ in return Foo{ as a composite type, and Foo( in return Foo( as a function call. I think the latter is definitely the right call, and the late(r) bindings / rebinding suggestion is not that great. I also do realize that this does add a little complexity to the parser, but it definitely affords a much higher quality of life for daily programmers

@gopherbot gopherbot added this to the Proposal milestone May 25, 2023
@jaloren
Copy link

jaloren commented May 25, 2023

What’s the practical use case for this in an actual code? I rarely see functions with more than four arguments. Anything that requires more usually means the function just takes a config struct.

@apparentlymart
Copy link

apparentlymart commented May 25, 2023

If I understand correctly, this proposed feature would make the parameter names chosen in the signature be part of the significant API of the package defining the named function type, whereas today those names are exclusively for documentation purposes and so authors can freely change them if they think of better names later based on user feedback.

I would personally find it surprising for it to be considered a breaking change to select a different name for an parameter in the signature, or to remove the names and use just the types. I worry that lots of package maintainers will make this mistake and learn about this unusual language feature only when they get a bug report saying that a caller's code is now broken.

If the information being elided were syntactically "closer" to the definition that implies that information then I might evaluate this tradeoff differently. For example, I feel good about the ability to omit the type part of a nested composite literal when the type is implied by another composite literal directly enclosing it, but I think allowing such an implication to flow from a package-level declaration into an expression inside an unrelated function -- which might not even be in the same package -- makes it far too easy for changes in one part of a large system to have confusing impact on other subsystems far from the declaration being changed, which is a maintenance hazard.

@apparentlymart
Copy link

Based on my understanding of the proposals process, I think this proposal should be based on the language change proposal template.

@seankhliao seankhliao added LanguageChange v2 A language change or incompatible library change WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. labels May 25, 2023
@seankhliao seankhliao modified the milestones: Proposal, Go2 May 25, 2023
@zephyrtronium
Copy link
Contributor

Not identical, but very similar to #30931.

@seankhliao seankhliao changed the title proposal: it should be easier to define functions of a named function type. affected/package: language change proposal: Go 2: short function declarations with named function type May 25, 2023
@seankhliao
Copy link
Member

see also #21498

@chewxy
Copy link
Author

chewxy commented May 25, 2023

What’s the practical use case for this in an actual code? I rarely see functions with more than four arguments. Anything that requires more usually means the function just takes a config struct.

Yeah you are right. The example I wrote was an extreme example for the sake of illustration. The actual code I wrote which led to opening of this issue looked something like this (it's long and filled with type params): func(expectations functional.Expectations[int, float64], indices *tensor.Dense[int], retVal *tensor.Dense[float64], right, left func(int, float64) (int, error)). I had 16 of these makeFoo equivalents to write.

Whether or not this is a code smell (the function signature probably is, I'm still in exploration mode), you gotta admit that with package names and long type names and potentially instantiations of generic data types, you will end up with very long function signatures. And yes while names can be shortened or aliased out, it's not always the most prudent thing to do.

I ended up writing a bit of elisp to generate the skeleton for my 16 makeFoo functions. But this is not ideal. Not everyone uses a programmable text editor.

@chewxy
Copy link
Author

chewxy commented May 26, 2023

Additionally, there's the issue of consistency. In Go, values of almost all the named composite types (arrays, maps, slices, structs, channels) can be instantiated by using its name, e.g.

type MyChan chan int
type SoC struct{ C MyChan }
type ListOfSoC []SoC
type MyMap map[int]ListOfSoC

... 

C := make(MyChan)
s := SoC{C}
l := ListOfSoC{s, s, s}
m := MyMap{0: l}

There's one composite type that is missing from having this ability: function types. For the sake of consistency, we should allow values of function types to be instantiated with their names too

@earthboundkid
Copy link
Contributor

For the sake of consistency, we should allow values of function types to be instantiated with their names too

You can do f := myFunc(nil). It's only illegal if you want to define the function body, which seems like a decent restriction since you need to name the arguments to use them.

If #21498 were accepted, I think that would be a good enough solution.

@seankhliao
Copy link
Member

This means a cosmetic change in the argument names of a func type definition becomes a breaking change, potentially in a far away place

@jarrodhroberson
Copy link

this is an awful proposal from a maintenance/comprehension/reading stand point.

"features" that do nothing but save keystrokes have exponential downsides when trying to maintain the code later, or read it to learn what it is doing. explicit is better than implicit. and competent IDEs like GoLand will auto generate the mapping by pressing CRTL+ALT+ENTER. whatever time implementing this would be much better spent on something way more useful like allowing arbitrary struct to be comparable by specifying a function for the struct.

@seankhliao seankhliao added WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. and removed WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. labels Sep 18, 2023
@gopherbot
Copy link

Timed out in state WaitingForInfo. Closing.

(I am just a bot, though. Please speak up if this is a mistake or you have the requested information.)

@gopherbot gopherbot closed this as not planned Won't fix, can't repro, duplicate, stale Oct 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
LanguageChange Proposal v2 A language change or incompatible library change WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Projects
None yet
Development

No branches or pull requests

8 participants