gno icon indicating copy to clipboard operation
gno copied to clipboard

feat: add support for type declarations on pointer types

Open ltzmaxwell opened this issue 1 year ago • 1 comments

support type decl upon pointer type:

package main

import "fmt"

// Define a base type
type Base int

// Declare a new type that is a pointer to the base type
type PtrToBase *Base

func main() {
	var b Base = 42      // Initialize a variable of the base type
	var p PtrToBase = &b // Initialize a variable of the new pointer type with the address of b

	fmt.Printf("The value of b is: %d\n", b)

	// Using the new pointer type
	fmt.Printf("The value pointed to by p is: %d\n", *p)

	// Modifying the value pointed to by p
	*p = 100
	fmt.Printf("The new value of b after modification through p is: %d\n", b)
}

// Output:
// The value of b is: 42
// The value pointed to by p is: 42
// The new value of b after modification through p is: 100

ltzmaxwell avatar Mar 05 '24 03:03 ltzmaxwell

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 47.79%. Comparing base (629c438) to head (a2521a5). Report is 75 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1733      +/-   ##
==========================================
+ Coverage   47.43%   47.79%   +0.35%     
==========================================
  Files         384      393       +9     
  Lines       61205    61891     +686     
==========================================
+ Hits        29032    29579     +547     
- Misses      29744    29829      +85     
- Partials     2429     2483      +54     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Mar 05 '24 03:03 codecov[bot]

@ltzmaxwell Have you had a chance to address @deelawn's comment?

zivkovicmilos avatar Mar 27 '24 12:03 zivkovicmilos

@ltzmaxwell Have you had a chance to address @deelawn's comment?

yes I'm on it, sorry for the delay.

ltzmaxwell avatar Mar 27 '24 12:03 ltzmaxwell

Nice one @ltzmaxwell. We just need to make sure that support for this feature is added in all places. I imagine there could be quite a few places that are currently casting to *PointerType without bothering to ensure it is using the pointer's base type now that the pointer type can be a declared type.

Take this code as an example:

package main

type BytePtr *byte

func main() {
	bs := []byte("hello")
	var p BytePtr = &bs[0]
	println(*p)
}

Running this will cause a panic. There is a line here that is incorrect: https://github.com/gnolang/gno/blob/master/gnovm/pkg/gnolang/op_expressions.go#L150

It should probably be changed to:

tv := TypedValue{T: bt.Elt}

to ensure it is using the base pointer type.

I haven't searched extensively but there may be other situations like this. This was one of the first hits that came up when I searched for .(*PointerType).

this is added, also some other tests, thank you @deelawn !

ltzmaxwell avatar Apr 09 '24 08:04 ltzmaxwell

Thanks for fixing the type assertion issues @ltzmaxwell. I can see one more thing that should be done to make this solution complete. Now that we support aliasing pointer types, we should also make the change to ensure that method receivers cannot have an underlying pointer type, as per the go spec https://arc.net/l/quote/mihfuukq, and also to avoid ambiguity for cases where this would currently be allowed. For example, this should fail during realm creation in the preprocessor with a message like invalid receiver type IntPtr (pointer or interface type):

package main

type IntPtr *int

var ip IntPtr = new(int)

func (p IntPtr) Int() int {
	return *p
}

func main() {
	println(ip.Int())
}

This could also be a separate PR. What do you think @petar-dambovaliev

deelawn avatar Apr 09 '24 16:04 deelawn

Thanks for fixing the type assertion issues @ltzmaxwell. I can see one more thing that should be done to make this solution complete. Now that we support aliasing pointer types, we should also make the change to ensure that method receivers cannot have an underlying pointer type, as per the go spec https://arc.net/l/quote/mihfuukq, and also to avoid ambiguity for cases where this would currently be allowed. For example, this should fail during realm creation in the preprocessor with a message like invalid receiver type IntPtr (pointer or interface type):

package main

type IntPtr *int

var ip IntPtr = new(int)

func (p IntPtr) Int() int {
	return *p
}

func main() {
	println(ip.Int())
}

This could also be a separate PR. What do you think @petar-dambovaliev

Nice! as always. I have applied a patch to address this issue. Please refer to the test files for further details and validation.

ltzmaxwell avatar Apr 10 '24 10:04 ltzmaxwell