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: add package ptr provide pointer functions #61082

Closed
leaxoy opened this issue Jun 30, 2023 · 14 comments
Closed

proposal: add package ptr provide pointer functions #61082

leaxoy opened this issue Jun 30, 2023 · 14 comments
Labels
Milestone

Comments

@leaxoy
Copy link

leaxoy commented Jun 30, 2023

Pointer is widely used in go, pointer also provide optional semantic, and We often need to deal with the default value of the pointer and the operation of the underlying data, such as: comparing the underlying data, etc.

Since we have generics now, so provide a package to solve those problems are useful and convenient.

Some possible APIs are as follows:

func zero[T any]() (v T) { return }

// Ref return reference of value
// this is convenient to deal with primitive literal data, 
// like: ptr.Ref(1000) -> *int without create a variable
func Ref[T any](t T) *T { return &t }
// Or function name `To`
func To[T any](t T) *T { return &t }

// Default return default value of type
func Default[T any]() *T {
	return Ref(zero[T]())
}

// ValueOr return value of pointer if not nil, else return default value.
func ValueOr[T any](v *T, d T) T {
	if v == nil {
		return d
	}
	return *v
}

// ValueOrZero return value of pointer if not nil, else return zero value.
func ValueOrZero[T any](v *T) T {
	return ValueOr(v, zero[T]())
}

// Compare compares two pointer. If both non-nil, compare underlying data,
// if both nil, return 0, non-nil pointer is always greater than nil pointer.
func Compare[T cmp.Ordered](l, r *T) int {
	if l != nil && r != nil {
		return cmp.Compare(*l, *r)
	}
	if l == nil && r == nil {
		return 0
	}
	if l != nil {
		return +1
	}
	return -1
}

// Equal test whether two pointer are equal. If both non-nil, test underlying data,
// if both nil, return true, else return false
func Equal[T comparable](l, r *T) bool {
	if l != nil && r != nil {
		return *l == *r
	} else if l == nil && r == nil {
		return true
	}
	return false
}
@gopherbot gopherbot added this to the Proposal milestone Jun 30, 2023
@seankhliao
Copy link
Member

seankhliao commented Jun 30, 2023

previously #38298 (comment)
related #45624

@iand
Copy link
Contributor

iand commented Jun 30, 2023

Most of these functions appear to be confusing pointer equality with equality of the things pointed to.

@earthboundkid
Copy link
Contributor

This is pretty similar to https://github.com/carlmjohnson/pointer. Why not just use my third party package instead of having it in the standard library?

@DeedleFake
Copy link

Why not just use my third party package instead of having it in the standard library?

I didn't know it existed until you posted it just now, nor would I have thought to go looking for a package like this. If I wanted a function like one of these, I'd just write it in whatever package I needed it in. I'd probably wind up writing it quite a few times across multiple projects.

I think some of the function names and godoc comments as proposed are confusing and/or misleading, but I think that I generally agree with the idea of the proposal. A lot of these are potentially useful in a lot of contexts and seem as good a fit for the standard library to me as the new cmp package is.

@ianlancetaylor
Copy link
Contributor

#45624 is a proposal to add this capability to the language directly.

@leaxoy
Copy link
Author

leaxoy commented Jul 1, 2023

@iand @Nasfame You are right, the name is also confused me, naming those functions is also one of the important topics, but functionality should be deterministic.

@leaxoy
Copy link
Author

leaxoy commented Jul 1, 2023

This is pretty similar to https://github.com/carlmjohnson/pointer. Why not just use my third party package instead of having it in the standard library?

@carlmjohnson

I didn't know about this library either until you pointed it out. Without it in std library, I prefer to implement it myself instead of searching on GitHub. It is commonly used but not complicated to implement. I also have an implementation at ptr, but how many people know about it until I bring it up?

@leaxoy
Copy link
Author

leaxoy commented Jul 1, 2023

I think some of the function names and godoc comments as proposed are confusing and/or misleading,

@DeedleFake

English is not my mother tongue, so there may be some expressions that are not clear in English. But these functional implementations are explicit.

@leaxoy
Copy link
Author

leaxoy commented Jul 1, 2023

#45624 is a proposal to add this capability to the language directly.

@ianlancetaylor

There is indeed some functional overlaps here, but cannot be completely replaced by #45624.

#45624 need language change, but this one don't need. Although it has been proposed for two years, there is still no key progress

This also provide other functions that #45624 not contains, such as handle default value, comparison and equality.

In addition, default value handle, in some cases, can replace the ternary operator. Consider the following situation:

func y(x *int) {
    v := 100
    if x != nil {
        v = *x
    }
}

// can simplify to 

func y(x *int) {
    v := ptr.ValueOr(x, 100)
}

@leaxoy
Copy link
Author

leaxoy commented Jul 1, 2023

  • All the constraints are not restricted to pointers!

@Nasfame

In go, we can't, but I think this should be another issue to discuss type constraint. I had initiated discussion at golang-nuts, but not many people have paid attention to this question, and did not get the reply from the official team. https://groups.google.com/g/golang-nuts/c/uGU7sbKqVZ4/m/9qxpPN8jAQAJ

@rsc
Copy link
Contributor

rsc commented Dec 4, 2023

Based on the discussion above, this proposal seems like a likely decline.
— rsc for the proposal review group

@rsc
Copy link
Contributor

rsc commented Dec 14, 2023

No change in consensus, so declined.
— rsc for the proposal review group

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Declined
Development

No branches or pull requests

9 participants