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/types: Export UntypedZero and Value #62124

Closed
griesemer opened this issue Aug 17, 2023 · 12 comments
Closed

proposal: go/types: Export UntypedZero and Value #62124

griesemer opened this issue Aug 17, 2023 · 12 comments
Assignees
Labels

Comments

@griesemer
Copy link
Contributor

Proposal

The go/types package shall export the following new declarations:

const UntypedZero BasicKind = 26

and

// A Value represents the predeclared value nil or zero.
// The specific value is identified by its type.
type Value struct {
    object
}

// For backward-compatibility.
// Deprecated.
type Nil = Value

such that go/types clients have easy access to the type and object for the new predeclared value zero (approved in #61372).

Discussion

The zero object can be found via Universe.Lookup("zero") and thus it's type is Universe.Lookup("zero").Type(). But we provide BasicKind values for all other predeclared types, so Typ[UntypedZero] will provide direct access to the type for zero. Importers may need access to the actual kind value.

Until Go 1.21, Go defined only a single predeclared value, nil. go/types used a special object, Nil to represent this value. Now that we have both nil and zero, rather than introduce a new object kind (say Zero), it makes sense to use a Value object to represent both. The distinguishing feature between nil and zero is their type, so no extra information needs to be stored with Value. We could simply use the Nil object for both, but it seems cleaner to rename it to Value and make Nil an alias for Value for backward-compatibility.

None of these changes are strictly necessary (Universe.Lookup("zero") will provide the zero object and type), but it makes sense to make these changes to keep thego/types API directly in sync with the language.

@griesemer griesemer added this to the Go1.22 milestone Aug 17, 2023
@griesemer griesemer self-assigned this Aug 17, 2023
@griesemer
Copy link
Contributor Author

cc: @findleyr

@findleyr
Copy link
Contributor

findleyr commented Aug 28, 2023

I think this is the simplest change implementing support for zero, and would allow us to migrate to the new Value Object type, which is probably what we would have chosen had the go/types API been designed with both nil and zero in the language.

However, I've thought about this a bit more since our brainstorming session, and am concerned that by not introducing a new node type, code could become subtly broken rather than panicking. For example, I grepped x/tools for types.Nil, and found this:
https://cs.opensource.google/go/x/tools/+/master:gopls/internal/lsp/semantic.go;l=497;drc=b35949e23814650f4c5caf482adbf63b6550fead

When encountering zero, that code would highlight the first three characters of zero in a different color, rather than panicking. I don't think that code is technically incorrect. The current documentation for types.Nil is Nil represents the predeclared value nil. Unfortunately that's a rather explicit API, and probably not one we can change in this way.

So, alas, we probably need a new object type Zero, along with the UntypedZero type.

@rogpeppe
Copy link
Contributor

I'm sure I don't have sufficient context here, but what is it about the identifier "Value" that suggests zero- or nil-ness ?

@griesemer
Copy link
Contributor Author

griesemer commented Aug 31, 2023

Not sure I interpret the question correctly, but if we introduce a new types.Object Value, it simply stands for some value (like Const stands for a constant, Func stands for a function, etc.). Since it's not a constant, we don't know the explicit value, but we know the type. So nil would be represented as a Value with Value.typ = types.Typ[UntypedNil], and zero would be represented as a Value with Value.typ = types.Typ[Untyped.Zero].

Currently, the CL makes Nil and alias of the new Value. Alternatively, we can keep Nil as is.

(I'd still prefer introducing Value as a general value object than having a Zero object.)

@findleyr
Copy link
Contributor

findleyr commented Aug 31, 2023

Per discussion, perhaps it's OK to use Value after all.

Rationale: types.Nil still means nil for code written for Go 1.21 and below. Tools seeking to support Go 1.22+ must be updated anyway. I think it's a compatibility gray area, and if it would be more ergonomic to make this change I think it's OK.

@adonovan
Copy link
Member

We could simply use the Nil object for both

I think that would be better: just add a doc comment to Nil saying that if its Type() is Typ[UntypedZero] then it represents zero. The new name Value is confusing because it sounds similar to TypeAndValue, and constant.Value, but is in fact unrelated.

@findleyr
Copy link
Contributor

findleyr commented Sep 20, 2023

I understand the concerns about the name Value, but it doesn't make a ton of sense to look up "zero" and get a Nil. I still think we should choose a new name, and make Nil an alias. The spec calls nil a "predeclared value", which is why Value is natural, but you're right that it is somewhat vague and overloaded.

Unfortunately, I can't think of anything better than Value. (PredeclaredValue and UntypedValue come to mind, but are too long and specific).

@griesemer griesemer removed this from the Go1.22 milestone Oct 2, 2023
@griesemer
Copy link
Contributor Author

Closing this as #61372 was declined and closed. We can always revive if needed.

@rsc
Copy link
Contributor

rsc commented Oct 3, 2023

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

@rsc rsc removed the Proposal-Hold label Oct 3, 2023
@willfaught
Copy link
Contributor

If this is active, shouldn't it be reopened?

@griesemer
Copy link
Contributor Author

@rsc's comment was probably from a script. We will remove this proposal from the active column in the next review. The reason for this proposal, issue #61372, was closed so this issue is moot.

@rsc
Copy link
Contributor

rsc commented Oct 5, 2023

This proposal has been declined as retracted.
— 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

6 participants