IsNil check. How?

[Alex Besogonov]

Hi!

I'm trying to solve the eternal "interface == nil" problem and I can't find a solution in this case: https://go.dev/play/p/DSik2kJ-gg4

It looks like casting to the base pointer type confuses the reflection package. Is there a way to solve this?

PS: PLEASE do add "nilptr" to the language proper.

[Axel Wagner]

On Wed, Mar 16, 2022 at 6:21 PM Alex Besogonov <alex.be...@gmail.com> wrote:

I'm trying to solve the eternal "interface == nil" problem and I can't find a solution in this case: https://go.dev/play/p/DSik2kJ-gg4

ISTM that you are discovering why this check just is not an interesting check to make in the first place. 

There is no meaning you can assign to "is the dynamic value of a non-nil interface nil", as `nil` might not even be a valid value for its dynamic type. It's also not a *helpful* check, because even non-nil pointers can cause a panic, if you call an associated method on them.

You *can* write a check to see if a value contains a pointer relatively easily using reflect:

https://go.dev/play/p/dDHp7KnoHHS

But it's not a very useful check, unless you specifically want to work further with the pointer using reflect (e.g. how json.Unmarshal stores a value in a pointer).

 

It looks like casting to the base pointer type confuses the reflection package. Is there a way to solve this?

PS: PLEASE do add "nilptr" to the language proper.

--
You received this message because you are subscribed to the Google Groups "golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/469b88b2-6d64-4bcd-8b92-18f9781c4fd3n%40googlegroups.com.

[Alex Besogonov]

Nope. Your solution doesn't work either: https://go.dev/play/p/DSik2kJ-gg4 (it doesn't crash, just falsely returns 'false').

What I'm trying to check is pretty straightforward: do I get the nil value passed to a function. All types are completely valid, the structure is correctly cast to the base interface type and its methods can be safely called: https://go.dev/play/p/xseNiCSB0uA

I don't see anything "uninteresting" in that. In a real program the interface is simply passed through an intermediate function, so I can't use an exact type.

That's a stupid part of the language and there's no good justification why this hasn't been yet fixed.

[Axel Wagner]

On Wed, Mar 16, 2022 at 7:02 PM Alex Besogonov <alex.be...@gmail.com> wrote:

Nope. Your solution doesn't work either: https://go.dev/play/p/DSik2kJ-gg4 (it doesn't crash, just falsely returns 'false').

That's your original code. I assume, though, you are trying to demonstrate that this doesn't return `true` when passed a nil-slice/func/chan/map. That's by design - the function does what it is designed to do, check if an interface value contains a nil-pointer. If you want something else, write a different function.

 

What I'm trying to check is pretty straightforward: do I get the nil value passed to a function.

The way to check that is `return v == nil`. What you seem to be interested in is "I want to write a function which checks if the dynamic type of a function is a pointer, slice, channel, function or map type and if so, returns if the dynamic value is nil". You can write that easily enough.

And before you link to another "this doesn't work either": Actually state what you are interested, then. A clear statement of your problem will lead to a clear path on how to write that code. 

 

All types are completely valid, the structure is correctly cast to the base interface type and its methods can be safely called: https://go.dev/play/p/xseNiCSB0uA

I don't see anything "uninteresting" in that. In a real program the interface is simply passed through an intermediate function, so I can't use an exact type.

The uninteresting part is *why* you want to answer this question. It is just unlikely to be a meaningful answer to the problem you are actually trying to solve. And you'll likely be happier with Go, if you try to adhere to its design and the patterns it lays out, instead of trying to fight them. 

 

That's a stupid part of the language and there's no good justification why this hasn't been yet fixed.

There are a many good justifications. Even if you disagree with the reasoning, it is there. And that kind of language won't get you very positive interactions.

 

On Wednesday, March 16, 2022 at 10:36:48 AM UTC-7 axel.wa...@googlemail.com wrote:

On Wed, Mar 16, 2022 at 6:21 PM Alex Besogonov <alex.be...@gmail.com> wrote:

I'm trying to solve the eternal "interface == nil" problem and I can't find a solution in this case: https://go.dev/play/p/DSik2kJ-gg4

ISTM that you are discovering why this check just is not an interesting check to make in the first place. 

There is no meaning you can assign to "is the dynamic value of a non-nil interface nil", as `nil` might not even be a valid value for its dynamic type. It's also not a *helpful* check, because even non-nil pointers can cause a panic, if you call an associated method on them.

You *can* write a check to see if a value contains a pointer relatively easily using reflect:

https://go.dev/play/p/dDHp7KnoHHS

But it's not a very useful check, unless you specifically want to work further with the pointer using reflect (e.g. how json.Unmarshal stores a value in a pointer).

 

It looks like casting to the base pointer type confuses the reflection package. Is there a way to solve this?

PS: PLEASE do add "nilptr" to the language proper.

--
You received this message because you are subscribed to the Google Groups "golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/469b88b2-6d64-4bcd-8b92-18f9781c4fd3n%40googlegroups.com.

--
You received this message because you are subscribed to the Google Groups "golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/75f9517f-c6ea-4a93-8c88-d1293b5c8c0en%40googlegroups.com.

[Alex Besogonov]

On Wednesday, March 16, 2022 at 12:29:07 PM UTC-7 axel.wa...@googlemail.com wrote:

On Wed, Mar 16, 2022 at 7:02 PM Alex Besogonov <alex.be...@gmail.com> wrote:

Nope. Your solution doesn't work either: https://go.dev/play/p/DSik2kJ-gg4 (it doesn't crash, just falsely returns 'false').

That's your original code. I assume, though, you are trying to demonstrate that this doesn't return `true` when passed a nil-slice/func/chan/map. That's by design - the function does what it is designed to do, check if an interface value contains a nil-pointer. If you want something else, write a different function.

I don't believe that's true. The function is passed an interface (Tester) that has a nil pointer part but a non-nil type. That type also happens to be a struct with an embedded pointer to another struct that implements the interface.

Here's a simplified version of the real code: https://go.dev/play/p/8LfgIGhd8GR - it compiles without any issues.

 

What I'm trying to check is pretty straightforward: do I get the nil value passed to a function.

The way to check that is `return v == nil`. What you seem to be interested in is "I want to write a function which checks if the dynamic type of a function is a pointer, slice, channel, function or map type and if so, returns if the dynamic value is nil". You can write that easily enough.

Except that you can not. In my case I have an interface that has the nil data part, and the program crashes because the reflect code doesn't know that I'm accessing the interface.

You can check my example, if you remove the nil check and use non-nil parameters, it would work just fine.

[Axel Wagner]

On Wed, Mar 16, 2022 at 10:19 PM Alex Besogonov <alex.be...@gmail.com> wrote:

On Wednesday, March 16, 2022 at 12:29:07 PM UTC-7 axel.wa...@googlemail.com wrote:

On Wed, Mar 16, 2022 at 7:02 PM Alex Besogonov <alex.be...@gmail.com> wrote:

Nope. Your solution doesn't work either: https://go.dev/play/p/DSik2kJ-gg4 (it doesn't crash, just falsely returns 'false').

That's your original code. I assume, though, you are trying to demonstrate that this doesn't return `true` when passed a nil-slice/func/chan/map. That's by design - the function does what it is designed to do, check if an interface value contains a nil-pointer. If you want something else, write a different function.

I don't believe that's true. The function is passed an interface (Tester) that has a nil pointer part but a non-nil type. That type also happens to be a struct with an embedded pointer to another struct that implements the interface.

Here's a simplified version of the real code: https://go.dev/play/p/8LfgIGhd8GR - it compiles without any issues.

Yes it compiles. Plenty of buggy Go code will compile. Here is another example of buggy interface-based code which compiles just fine. We can't prevent users from writing buggy code.

It seems in this case, the bug is storing such a value in that interface. The solution is to not do that.

Note that in your case, the dynamic value of that interface is not nil. It's a struct value. Structs can not be nil. Your example is a conclusive demonstration that "I want to check that the dynamic value in an interface value is nil" is a pointless check. The dynamic value might not be nil, but that doesn't make it useful.

Except that you can not. In my case I have an interface that has the nil data part, and the program crashes because the reflect code doesn't know that I'm accessing the interface.

The reflect code panics, because you are comparing a struct value to nil. Struct values can not be nil.

Again, just write different reflect code then. Reflect gives you the capability to write any dynamic check you want. If you want to check that an embedded pointer field is not-nil, then write the reflect code to check for that. I don't really understand what you are trying to check. But you can implement whatever heuristic you want, using reflect.

But really, the solution here is not to "check better". It's to not store invalid implementations of an interface in the interface. That's what it comes down to in almost every case this question is asked. The problem is almost always that there is an invalid implementation of an interface stored in it. If we'd change the language to make comparison of interfaces with nil different, that will almost always simply hide that bug, while not actually making the program any better.

There *are* valid uses for this check - json.Unmarshal is one such. But those cases need reflection anyway. So reflect is a far more sensible and flexible way for programmers to implement whatever check they need, than changing the language.

You can check my example, if you remove the nil check and use non-nil parameters, it would work just fine.

Okay. Then it seems you have your solution - "remove the nil check and use non-nil parameters".

 

--
You received this message because you are subscribed to the Google Groups "golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/70a80a76-44b5-4af3-9eb5-2d8794e4d3c4n%40googlegroups.com.

[Alex Besogonov]

Here's a simplified version of the real code: https://go.dev/play/p/8LfgIGhd8GR - it compiles without any issues.

Yes it compiles. Plenty of buggy Go code will compile. Here is another example of buggy interface-based code which compiles just fine. We can't prevent users from writing buggy code.

This is not a buggy code. This is a buggy language spec.

It seems in this case, the bug is storing such a value in that interface. The solution is to not do that.

No. The bug is in Go’s spec that treats values and pointers differently when coercing values to an interface type.

Note that in your case, the dynamic value of that interface is not nil. It's a struct value. Structs can not be nil.

Incorrect. Go language allows coercing a struct with an embedded pointer to an interface. Spec:

"If S contains an embedded field *T, the method sets of S and *S both include promoted methods with receiver T or *T."

Except that you can not. In my case I have an interface that has the nil data part, and the program crashes because the reflect code doesn't know that I'm accessing the interface.

The reflect code panics, because you are comparing a struct value to nil. Struct values can not be nil.

No. I’m comparing AN INTERFACE, not a struct. The interface’s provenance is _through_ a struct that has an embedded pointer, but this should not matter to the code that accepts the interface.

Would you argue that these statements should behave differently: '1 + 1 == 2’ and ‘2 == 2’? Because you argue that ‘1 + 1’ is not ‘2’ and should not work this way.

Again, just write different reflect code then.

Which? Go seems to be losing the type of the variable. It’s not possible to say “get me a value as THIS interface type”.

Reflect gives you the capability to write any dynamic check you want. If you want to check that an embedded pointer field is not-nil, then write the reflect code to check for that. I don't really understand what you are trying to check. But you can implement whatever heuristic you want, using reflect.

I’m trying to check that a freaking interface pointer part is nil. That’s it.

But really, the solution here is not to "check better". It's to not store invalid implementations of an interface in the interface.

I gave you the real example, somewhat simplified.

There *are* valid uses for this check - json.Unmarshal is one such. But those cases need reflection anyway. So reflect is a far more sensible and flexible way for programmers to implement whatever check they need, than changing the language.

OK. How do I make this check using reflection? I can’t find a way to do it. The only way is to use `unsafe` to manually hack into the low-level interface  representation to extract the pointer part.

You can check my example, if you remove the nil check and use non-nil parameters, it would work just fine.

Okay. Then it seems you have your solution - "remove the nil check and use non-nil parameters”.

Not feasible. It would require huge amounts of code in call sites.

[Axel Wagner]

On Wed, Mar 16, 2022 at 11:25 PM Alex Besogonov <alex.be...@gmail.com> wrote:

This is not a buggy code. This is a buggy language spec.

You might be best served using a different language then.

No. The bug is in Go’s spec that treats values and pointers differently when coercing values to an interface type.

I don't believe it does. Only insofar as to check whether or not the methods have pointer or value receivers.

 

Note that in your case, the dynamic value of that interface is not nil. It's a struct value. Structs can not be nil.

Incorrect. Go language allows coercing a struct with an embedded pointer to an interface.

That has absolutely no relevance to the question of whether or not structs can be nil.

The spec says what types can be compared to `nil` and structs are not among them:

https://go.dev/ref/spec#Comparison_operators

It also says what types `nil` can be assigned to and structs are not among them:

https://go.dev/ref/spec#Assignability

It also says what types have zero value `nil` and structs are not among them:

https://go.dev/ref/spec#The_zero_value

Therefore, comparisons of structs to `nil` are invalid, as structs can never be nil. No matter whether you do that statically or using reflect: https://go.dev/play/p/Gy19rSvenbr

 

The reflect code panics, because you are comparing a struct value to nil. Struct values can not be nil.

No. I’m comparing AN INTERFACE, not a struct.

You are using `reflect` to inspect the dynamic value of that interface. That dynamic value is a struct.

The interface’s provenance is _through_ a struct that has an embedded pointer, but this should not matter to the code that accepts the interface.

I completely agree with this. That's why it is so important not to store invalid values in an interface. The code using that interface has no reasonable way to check if the value is valid. So it has to rely on the caller to only provide valid implementations.

 

Would you argue that these statements should behave differently: '1 + 1 == 2’ and ‘2 == 2’? Because you argue that ‘1 + 1’ is not ‘2’ and should not work this way.

That is a false equivalency. Interfaces are not integer types. Neither are structs. Or pointers.

The Go type system is more complicated than that. Denying that it exists is not helpful for your cause.

Again, just write different reflect code then.

Which? Go seems to be losing the type of the variable. It’s not possible to say “get me a value as THIS interface type”.

Here is code which checks your specific case:

https://go.dev/play/p/qs9VjaQDVyZ

Of course, that check is non-sensical. It catches your specific case, but it won't catch

func (Boom) Lock() { panic("boom") }

And if you write code to catch that, it won't catch

func (Boom) Lock() { if time.Now().Weekday() == time.Tuesday { panic("boom") } }

And if you write code to catch that, it won't catch

func (Boom) Lock() {

    if IsNil(Boom{}) {

        return

    }

    panic("boom")

}

It is categorically impossible to write a check which detects whether a method call panics, without solving the halting problem - and solving the halting problem is categorically impossible.

That's why it's not reasonable to even try. The code receiving the interface should simply assume that calling methods is valid and it is the callers responsibility not to store invalid implementations in an interface. That's the only sensible way to handle this.

 

I’m trying to check that a freaking interface pointer part is nil. That’s it.

There is no such thing as "a freaking interface pointer part". Again, please be precise in what you are trying to do.

Your problem, as far as I understand it, is that you are storing a struct-value in an interface, which embeds a pointer. That pointer type implements an interface. But that implementation is not valid, if the pointer is nil.

You are then assigning the zero-value of that struct to an interface. The embedded pointer field of that struct is a nil-pointer. Thus it is not valid, as the implementation requires the pointer to be non-nil, but it is still an implementation of the interface the compiler accepts - just as it would accept the nil-pointer of *BasicNode.

That situation is easy enough to understand. But I'm not sure what you are trying to check from then on.

- If you want to check if the interface is not nil, you can do that. But checking an interface for nil-ness checks if it contains a dynamic value and this interface *has* a dynamic value, so the check doesn't help.

- If you want to check if the dynamic type can be nil and the dynamic value is nil, I gave you that code above: https://go.dev/play/p/X_pHOrxxago but it won't get you very far, because the dynamic value can not be nil, as the dynamic type is a struct type

- If you want to check if the dynamic value is a struct, which embeds a `*BasicNode`, which is nil, I gave you the code for that in this message. But that seems kind of silly, TBQH, because it seems overly specific.

- If you want to check if the dynamic value of the interface is a valid implementation (i.e. does not panic) you can't do that, as it would require solving the halting problem, which is impossible - not because of a short-coming of Go, but because of fundamental mathematical principles of how humanity does computation.

What I don't understand is why you don't take the simple route - not storing an invalid implementation in the interface in the first place. It seems you know what the bug is and where it is and could simply prevent it, but you insist on hiding it instead. I don't understand why.

The situation, as far as I understand it, is similar to someone posting that io.Copy panics if called with a Boom Reader and demands that an `if r == nil` check before `io.Copy` should detect that `Read` panics. That seems an outlandish request. But it's not very different from demanding that your `IsNil` code should detect that the implementation in the interface is an invalid struct.

 

But really, the solution here is not to "check better". It's to not store invalid implementations of an interface in the interface.

I gave you the real example, somewhat simplified.

Then this is the real example with the fix applied: https://go.dev/play/p/04H1KenHjRV

 

Okay. Then it seems you have your solution - "remove the nil check and use non-nil parameters”.

Not feasible. It would require huge amounts of code in call sites.

That's unfortunate. Under those premises, I'm afraid I can't really help you. The best answer I think I can give, is that ISTM you've been coded into a corner, where you have a bug-ridden codebase at hand which can not be fixed.

[Alex Besogonov]

The reflect code panics, because you are comparing a struct value to nil. Struct values can not be nil.

No. I’m comparing AN INTERFACE, not a struct.

You are using `reflect` to inspect the dynamic value of that interface. That dynamic value is a struct.

No. It should be a POINTER to a struct, which would have been fine. Instead Go's reflection incorrectly latches on the type of the embedding structure.

 

The interface’s provenance is _through_ a struct that has an embedded pointer, but this should not matter to the code that accepts the interface.

I completely agree with this. That's why it is so important not to store invalid values in an interface. The code using that interface has no reasonable way to check if the value is valid. So it has to rely on the caller to only provide valid implementations.

The code is NOT using anything incorrect. The interface is obtained through a pointer to a struct, which is completely valid. The only problem is that this pointer is embedded inside a struct.

If you rewrite the code to use a named field, it would work. This means that the language behaves differently depending on a field having a name.

 

But really, the solution here is not to "check better". It's to not store invalid implementations of an interface in the interface.

I gave you the real example, somewhat simplified.

Then this is the real example with the fix applied: https://go.dev/play/p/04H1KenHjRV

The real example would look like this:  https://go.dev/play/p/mwUfh4_RBUU - you do see why it's a tad problematic?

[Patrick Smith]

On Wed, Mar 16, 2022 at 3:25 PM Alex Besogonov <alex.be...@gmail.com> wrote:

Note that in your case, the dynamic value of that interface is not nil. It's a struct value. Structs can not be nil.

Incorrect. Go language allows coercing a struct with an embedded pointer to an interface. Spec:

"If S contains an embedded field *T, the method sets of S and *S both include promoted methods with receiver T or *T."

Consider this code:

interface Fooable { Foo() }

type T struct {}

func (*T) Foo() {}

type S struct { *T }

func bar(f Fooable) { f.Foo() }

func baz() {

   var s S

   bar(s)

}

As far as I can tell, you seem to think that the call bar(s) is equivalent to bar(s.T). In other words, what is passed to bar is an interface value containing a copy of s.T. This is not correct.

What actually happens is equivalent to the compiler generating a method

var (s S) Foo() { s.T.Foo() }

so that S actually implements the interface Fooable. And the call bar(s) passes to bar an interface value containing a copy of s itself.

[Axel Wagner]

On Thu, Mar 17, 2022 at 12:18 AM Alex Besogonov <alex.be...@gmail.com> wrote:

No. It should be a POINTER to a struct, which would have been fine. Instead Go's reflection incorrectly latches on the type of the embedding structure.

Your code says:

var n3 Node
LockObjects(n2, n3, n1)

You are thus storing a `Node`, which is a struct value, in a `Lockable`. It's not a pointer.

And the `reflect` package behaves correctly.

 

The code is NOT using anything incorrect.

We apparently have to agree to disagree. To me, passing around an interface value where every method call panics is a clear example of incorrect code. It is like writing a Boom io.Reader and claiming that the code is correct and the language is wrong for panicing when using it.

The interface is obtained through a pointer to a struct, which is completely valid. The only problem is that this pointer is embedded inside a struct.

If you rewrite the code to use a named field, it would work. This means that the language behaves differently depending on a field having a name.

Well, yes. That's the point of embedding, after all. That is working as intended.

The real example would look like this:  https://go.dev/play/p/mwUfh4_RBUU - you do see why it's a tad problematic?

Not really, no. It doesn't seem to change the basic properties of the code.

I don't know what the return type of `store.GetNode` is:

- If it is `Lockable`, it clearly should not return a `Node` with a nil `*BasicNode` field, as that is an invalid implementation of `Lockable`.

- If it is `Node`, then you clearly shouldn't check `n1 == nil`, but perhaps `n1 == nil || n1.BasicNode == nil`, before assigning it to `Lockable` (in the call to `LockObjects`).

Either way, some piece of code is assigning an invalid `Node` value to a `Lockable`. That code must be fixed, to no longer do that. Neither of these pieces of code seems particularly hard to fix.

--
You received this message because you are subscribed to the Google Groups "golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/58d5770b-4888-4980-8f9e-c96310d57490n%40googlegroups.com.

[Axel Wagner]

FWIW I understand that I come off as unsympathetic, insisting that you should fix the incorrect code, even though it is hard. But I am very sympathetic. I know perfectly well what it means to inherit a messy code base full of bugs and be overwhelmed by the amount of effort needed to fix it.

But clearly, the solution can't be to change the language on a whim. Changing the language is even harder and it affects even more people and requires us to touch even more code - by several orders of magnitude. And that's assuming we could even agree on what to change.

That's why I'm insistent that the solution has to be to fix your code. The language as it is can work just fine. There are many programmers using it very effectively for very large projects. It does require working with it, not against it. And sometimes that is hard and it means having to touch a lot of code someone else wrote, to fix a problem they unknowingly introduced.

It's difficult, but the alternatives are even more so.

[Axel Wagner]

Let me make one more concrete suggestion:

On Thu, Mar 17, 2022 at 12:18 AM Alex Besogonov <alex.be...@gmail.com> wrote:

The real example would look like this:  https://go.dev/play/p/mwUfh4_RBUU - you do see why it's a tad problematic?

Here is how you could write that, assuming `GetNode` returns a `*Node`:

https://go.dev/play/p/7oGL2F1QgON

It avoids having to write nested conditionals, while also not storing invalid Nodes in a `Lockable`.

This is just to illustrate that there are sensible middle-grounds, avoiding storing an invalid `Lockable` in a place where you still have the concrete type information, while still writing reasonable convenient code.

The point is just that the check for validity should happen *before* assigning the `Node` to a `Lockable`, not after. If you have to do it in `store.GetNode` (or file a bug against its owners), because that's where the assignment happens, then that's where it should happen. If it's something you do in your code, it should happen at that point.

Find wherever you assign a `Node` to a `Lockable` and insert a check there. If there are many such places, you can write a helper-function to make it easier, e.g.

func IsLockable(n *Node) bool { return n != nil && n.BasicNode != nil }

[Axel Wagner]

Also, I kind of assume that you have little control over the types (given that you say it's infeasible to touch all places where it's created) but the simplest solution might, of course, be to make the zero value of Node implement Lockable correctly:

type Node struct {
    BasicNode // Note: By not using a pointer, this can not be nil.
    payload int32
}

Now, `*Node` implements `Lockable` correctly (but `Node` itself doesn't, as `BasicNode`s methods have pointer receivers).

If you can do that it might side-step a lot of your problems. But if you can do that and if the result would still be semantically correct depends, of course, on what the meaning behind your types is. Which I don't know.