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: noncompare keyword to make structs non comparable #67196

Closed
4 tasks
Chig-Beef opened this issue May 6, 2024 · 9 comments
Closed
4 tasks

proposal: Go 2: noncompare keyword to make structs non comparable #67196

Chig-Beef opened this issue May 6, 2024 · 9 comments
Labels
LanguageChange Suggested changes to the Go language Proposal Proposal-FinalCommentPeriod v2 An incompatible library change
Milestone

Comments

@Chig-Beef
Copy link

Chig-Beef commented May 6, 2024

Go Programming Experience

Intermediate

Other Languages Experience

Python, C#, JS,

Related Idea

  • Has this idea, or one like it, been proposed before?
  • Does this affect error handling?
  • Is this about generics?
  • Is this change backward compatible? Breaking the Go 1 compatibility guarantee is a large cost and requires a large benefit

Has this idea, or one like it, been proposed before?

No (not that I know of)

Does this affect error handling?

No, if implemented correctly it should be identical

Is this about generics?

No

Proposal

Going through the codebase for ebitengine I noticed this code.

type ColorM struct {
	impl affine.ColorM

	_ [0]func() // Marks as non-comparable.
}

I thought this was strange, but understood what was happening. This issue here is that I don't think everybody would be able to understand what's happening especially without that comment. The fact is that this is the easiest way to do this that I know of, and it doesn't look great. It also doesn't seem very Go-like in my opinion, as it is very unreadable (without prior-knowledge or comment) and most Go code is relatively easy to understand.

The proposal is to create a way to simplify marking structs as non-comparable. I personally don't know what this should look like, but it could be something like this.

type Example noncompare struct {

}

Switch out the keyword for a nicer one (just an example). This is only one suggestion though, but I don't think I'm the person to ask for the most Go-like solution.

Language Spec Changes

A new keyword would be added to mark structs as non-comparable (or an alternative solution).

Informal Change

It is obtuse to mark structs as non-comparable, and therefore there should be a simpler way (whether with a keyword or not) to mark them as such.

Is this change backward compatible?

Maybe, depending on how it is implemented. With a keyword as suggested, this wouldn't be backwards compatible, however, there are definitely solutions that are.

Orthogonality: How does this change interact or overlap with existing features?

If this change occurs it would make it easier (especially for new programmers of Go) to read the code and understand it. It would also get rid of unnecessary comments. I wouldn't know how to measure that.

Would this change make Go easier or harder to learn, and why?

Very much easier.

type ColorM struct {
	impl affine.ColorM

	_ [0]func() // Marks as non-comparable.
}

In the above code you either have to treat the line as a black box that just magically marks structs as non-comparable, or learn what each individual part means.
A keyword would be explainable to its full extent with one sentence.

Cost Description

My worry is that we're adding another keyword to the language. To get around that you could scope the search for that keyword only in that one context, so that it doesn't break backwards compatibility, however, that may make it a more complex issue.

Changes to Go ToolChain

gofmt and gopls would definitely be affected, but other than that I don't think much more.

Performance Costs

In regards to compiler performance, I wouldn't expect it to be too expensive, as it's one extra token (depending on implementation). Runtime cost should be nothing (if not faster).

Prototype

type Example1 noncompare struct {

}
@Chig-Beef Chig-Beef added LanguageChange Suggested changes to the Go language Proposal v2 An incompatible library change labels May 6, 2024
@gopherbot gopherbot added this to the Proposal milestone May 6, 2024
@lucastomic
Copy link

I agree that that code you shared may be difficult to understand without the comment. However, I am not sure that it is necessary to change the syntax of the language, since a simpler version of the code can be achieved using composition

type MyStruct struct {
	val int
	NotComparable
}

type NotComparable struct {
	_ []int
}

func main() {
	a := MyStruct{val: 1}
	b := MyStruct{val: 1}
	fmt.Println(a == b) //compilation error
}

@Chig-Beef
Copy link
Author

That's a nice one, I do like that and just reading it I think it's pretty understandable. It still just feels like the language is slightly punishing you by being obtuse, which in most cases is the right thing to do, but marking things as non-comparable is completely valid. Also, using _ []int or _ [0]func() isn't a noop in terms of memory.

package main

import (
	"fmt"
	"unsafe"
)

type NotComparable struct {
	_ [0]func()
}

type UniqueByte struct {
	value byte
	NotComparable
}

type UniqueByteComp struct {
	value byte
	dummy byte
}

func main() {
	fmt.Println(unsafe.Sizeof(NotComparable{}))
	fmt.Println(unsafe.Sizeof(UniqueByte{}))
	fmt.Println(unsafe.Sizeof(UniqueByteComp{}))
	fmt.Println(unsafe.Sizeof(byte(0)))
}
/*
Output:
0
16
2
1
*/

Although it looks like that on its own the NotComparable type has no effects on memory by itself, it does start to show. Using _ []int also takes up more memory than _ [0]func(). Since whether something can be compared or not is governed by its type, it should be possible to deal with non-comparable variables at compile time, just as it is with slices. 14 bytes isn't much, but I get concerned with having larger slices of non-comparable items and then dealing with gc? I also don't know the effects on time performance, but if it's having to allocate that little extra memory it's definitely non-zero, which I usually wouldn't have an issue with but in this case it's unnecessary.

@seankhliao seankhliao changed the title proposal: Better way to make structs as non-comparable proposal: Go 2: noncompare keyword to make structs non comparable May 6, 2024
@earthboundkid
Copy link
Contributor

earthboundkid commented May 6, 2024

Zero length elements should go first in the struct to prevent unnecessary padding:

type UniqueByte struct {
	NotComparable
	value byte
}
Output:
0
8
2
1

@Chig-Beef
Copy link
Author

That's cool, does that work with multiple "flags," say we had use for more things like this, if I had 3 at the top, would the logic work for all 3 or just the top one? There is still extra (although yes, not much) redundant memory being created here.

fmt.Println(unsafe.Sizeof([32_768]UniqueByte{}))
fmt.Println(unsafe.Sizeof([32_768]UniqueByteComp{}))

/*
Output:
262144
65536
*/

I don't think a new keyword would be a good option though, I can definitely see why people don't want that. Is there are a better way to implement this? Or is it too little gain for such a large change? Especially if it breaks compatibility.

@ianlancetaylor
Copy link
Member

If we adopt #66408, we could add

package structs

type NonCompare [0]func()

I think this is a very rare need. I don't see any need for a new keyword here.

@Chig-Beef
Copy link
Author

That's probably the best way we could be doing it with what we have now and from what I saw in the issue it look like you can add a lot of "flags" in there.
The main issue I originally had was readability, and I think for the most part this fixes it because you don't see the [0]func() until you go looking for it, at which point you're probably ready to learn what it means.
My last issue would be this

type UniqueByte struct {
	structs.NonCompare
	value byte
}

f := UniqueByte{0}

Which results in an error. With the current solutions you can't create a struct this way (without specifying fields), because it still treats the flag as a property (because it is).

I don't mind that too much though, because I do like specifying the field names, unless it's a small struct (like an x, y, z position).

Also the fact that order of property definitions affects the size of a struct in memory seems crazy to me, but I think I might be thinking too hard about it (':

@bjorndm
Copy link

bjorndm commented May 8, 2024

This is actually pretty common since the Go protobuf code uses this kind of pseudo directives: https://github.com/protocolbuffers/protobuf-go/blob/master/internal/pragma/pragma.go But, #66408 is likely the correct solution.

@ianlancetaylor
Copy link
Member

Based on the discussion and the emoji voting, this is a likely decline. Leaving open for three weeks for final comments.
— ianlancetaylor for the language proposal review group

@ianlancetaylor
Copy link
Member

No further comments.

@ianlancetaylor ianlancetaylor closed this as not planned Won't fix, can't repro, duplicate, stale Jun 5, 2024
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

6 participants