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: add sugar multiple keys to map to be tuples #63431

Closed
Jorropo opened this issue Oct 7, 2023 · 4 comments
Closed

proposal: Go 2: add sugar multiple keys to map to be tuples #63431

Jorropo opened this issue Oct 7, 2023 · 4 comments
Labels
LanguageChange Suggested changes to the Go language Proposal Proposal-FinalCommentPeriod v2 An incompatible library change
Milestone

Comments

@Jorropo
Copy link
Member

Jorropo commented Oct 7, 2023

Author background

  • Would you consider yourself a novice, intermediate, or experienced Go programmer? experienced
  • What other languages do you have experience with? python, lua, assembly, ...

Related proposals

  • Has this idea, or one like it, been proposed before? a quick is:issue label:proposal maps in:title search suggest to me no
    • If so, how does this proposal differ?
  • Does this affect error handling? No
    • If so, how does this differ from previous error handling proposals?
  • Is this about generics? No
    • If so, how does this relate to the accepted design and other generics proposals?

This is based with concepts of #63221 (but could be refactored to not be).
In case #63221 is refused then I think this is especially valuable.

Proposal

  • What is the proposed change?
    Allow to define multiple types in the keys of maps:
type edges map[string, string]struct{}

And have it be equivalent to creating a tuple of the same types:

type edges map[struct(string, string)]struct{}

Indexing into the map would be sugar for pack:

func (m edges) Link(a, b) {
 m[a, b] = pack()
}

func (m edges) IsValidPath(path ...string) bool {
 a := path[0]
 for _, b := range path {
   if _, ok := m[a, b]; !ok {
    return false
   }
   a = b
 }
 return bool
}

Would be:

func (m edges) Link(a, b string) {
 m[pack(a, b)] = pack()
}

func (m edges) IsValidPath(path ...string) bool {
 a := path[0]
 for _, b := range path {
   if _, ok := m[pack(a, b)]; !ok {
    return false
   }
   a = b
 }
 return bool
}

Lastly maps literals would get the same treatment:

m := edges{"Nantes", "Paris": pack()}

Into:

m := edges{pack("Nantes", "Paris"): pack()}
  • Who does this proposal help, and why?
    Current alternatives are:
    • Create an anonymous struct for the key (or use an array if the usecase fits for it).
      Which works but is verbose.
    • Use proposal: spec: tuples as sugar for structs #63221, this is not yet accepted and is still a more verbose.
    • Use nested maps, this gives a good enough syntax m[a][b] but is very awkward to deal with when considering keys that can be missing, awkward collection when deleting elements and adding elements which need to check if the inner map exists. This gets very bad to use fast when using lots of elements in the key. And performance is in almost all cases worst. *This fits unique edge cases you need to iterate over an inner map, or pass the inner map around.
  • Please describe as precisely as possible the change to the language.
  • What would change in the language spec?
    • see above
  • Please also describe the change informally, as in a class teaching Go.

Ok so imagine you just explained and demonstrated how maps exists.

Well you can have multiple keys:

m := map[int, int, int]block{
  0, 0, 0: stone,
  0, 1, 0: dirt,
}
m[0, 2, 0] = water
fmt.Println(m[0, 1, 0])
  • Is this change backward compatible? Yes
    • Breaking the Go 1 compatibility guarantee is a large cost and requires a large benefit.
      Show example code before and after the change.
    • Before
    • After
  • Orthogonality: how does this change interact or overlap with existing features? It makes maps with multi-factorial keys more readable and easier to use.
  • Is the goal of this change a performance improvement? No
    • If so, what quantifiable improvement should we expect?
    • How would we measure it?

Costs

  • Would this change make Go easier or harder to learn, and why? I think this is very slightly harder, I belive map[string, float64, int]complex128 and m["a", 42, 1337] are self explanatory however in the rare cases someone would try to integrate this with iteration and encoding/json (through reflect) this would yield surprising results, at least if theses aren't updated to have special behavior for tuples.
  • What is the cost of this proposal? (Every language change has a cost). More lines in the spec. Require the compiler to read and understand map keys and canonicalize them into single type forms.
  • How many tools (such as vet, gopls, gofmt, goimports, etc.) would be affected? I don't know, this could range to all of them to few of them depending on whether or not updating go/parser would be enough.
  • What is the compile time cost? None
  • What is the run time cost? None
  • Can you describe a possible implementation? Update the parser following rules described above, when walking the AST right after binding types to names run proposal: spec: tuples as sugar for structs #63221 packing (create anonymous structs with F0, F1, ... Fn names) on map keys and map indexes with multiple entries.
  • Do you have a prototype? (This is not required.) No
@Jorropo Jorropo added LanguageChange Suggested changes to the Go language Proposal v2 An incompatible library change labels Oct 7, 2023
@gopherbot gopherbot added this to the Proposal milestone Oct 7, 2023
@Jorropo
Copy link
Member Author

Jorropo commented Oct 7, 2023

Thought:
Maybe delete(m, a, b, c) should act like delete(m, pack(a, b, c)) ?

@apparentlymart
Copy link

This does seem like some interesting syntax sugar that might help in some specific situations, but personally I think we should wait to see how #63221 fares first, since this seems like essentially a further layer of sugar on top of that. If that other more general proposal were not accepted, I think it would be hard to justify accepting this one that is effectively a less orthogonal version of the same idea (limited only to maps).

Even if that other proposal is accepted, I would still suggest waiting until it has been implemented and available for some time to see how often situations like map[struct(string, string)]string arise in real-world code.

Any "syntactic sugar" that hides something has the cost of adding another thing that a new Go developer might find in an existing codebase and not know its name to learn about it. I think that can be justified if a particular pattern appears often enough that a new developer is likely to encounter it often and probably have access to a colleague who can help explain what it means, but for less-frequently-used constructs it's more likely that someone could go several years without encountering it, and then encounter it for the first time when the original author is not around to explain it, leaving them stuck.

Of course that's a tradeoff, not a hard rule. But it's a tradeoff that's easier to make with the experience of seeing how the non-sugared version arises in practice, and it's not really possible to do that when it's dependent on another second level of syntax sugar that isn't available yet either.

With that said, I don't really have a strong opinion on the proposal itself, only on the "meta" that it seems a little early to get good data on whether it would be useful and thus make a decision about it.

@findleyr
Copy link
Contributor

findleyr commented Nov 1, 2023

While this is an interesting idea, it is a rather narrow use of tuples. If Go were to accept this syntax, it would be more consistent to also allow tuples elsewhere (and this may be required in order to work with range).

It is trivial to define a local struct type where required. Therefore, marking this as likely decline. Leaving open for four weeks for additional comments.

@adonovan
Copy link
Member

adonovan commented Dec 6, 2023

There were no further comments, and no change in consensus.

@adonovan adonovan closed this as completed Dec 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
LanguageChange Suggested changes to the Go language Proposal Proposal-FinalCommentPeriod v2 An incompatible library change
Projects
None yet
Development

No branches or pull requests

5 participants