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

fix: addressability #2731

Open
wants to merge 41 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
c8462ec
preprocessor addressability work
deelawn Aug 27, 2024
ef97274
fix selector isaddressable
deelawn Aug 27, 2024
7fa9f50
fix starexpr isaddressable
deelawn Aug 27, 2024
08563c3
fix sliceexpr addressability
deelawn Aug 27, 2024
3b78ae9
fix typeassertexpr addressability
deelawn Aug 27, 2024
a0e13b9
fix callexpr addressable return types
deelawn Aug 27, 2024
ae55b84
mark append and new results as addressable
deelawn Aug 27, 2024
990017d
fix selector addressability
deelawn Aug 27, 2024
3621dc7
make string indexes non-addressable
deelawn Aug 28, 2024
9faf3df
addressability file tests
deelawn Aug 28, 2024
8671ea5
prohibit taking address of imported typed constant
deelawn Aug 28, 2024
816dbd6
more tests
deelawn Aug 28, 2024
77caea9
fix type assertion addressability and add tests
deelawn Aug 29, 2024
89a041c
make sure to use base types when considering addressability
deelawn Aug 29, 2024
9956c26
move tests
deelawn Aug 29, 2024
6940714
fix index and selector addressability
deelawn Aug 29, 2024
3f4f1b6
introduce concept of addressability not applicable
deelawn Aug 29, 2024
c16e5e9
fixed test
deelawn Aug 29, 2024
ac79f00
fixed test
deelawn Aug 29, 2024
df764cd
fix test
deelawn Aug 30, 2024
af12ca1
func lit tests
deelawn Aug 30, 2024
e498f1c
index, map, and call expr fixes and tests
deelawn Aug 30, 2024
bee868c
addressability consistentcy fixes
deelawn Aug 30, 2024
9698461
consolidate simplest block path traversal
deelawn Sep 4, 2024
b284b56
fix comment
deelawn Sep 6, 2024
dfe024f
Removed test and renamed other
deelawn Sep 12, 2024
0e80801
move and rename addressability constants
deelawn Sep 19, 2024
804c17e
remove assertExpr
deelawn Sep 19, 2024
44cc298
clarifying comment
deelawn Sep 19, 2024
548af87
fixed comment
deelawn Sep 19, 2024
8bec242
panic when calling addressability on type expressions
deelawn Sep 21, 2024
8fe3b37
fixed tesst
deelawn Sep 21, 2024
26e8529
added comment
deelawn Sep 21, 2024
3c120ae
Merge branch 'master' into fix/addressability
deelawn Sep 21, 2024
feee13d
fixed test
deelawn Sep 21, 2024
b33f0b0
fix double reference addressability and add comments
deelawn Sep 21, 2024
a8827ff
fixed test
deelawn Sep 21, 2024
71afb10
corrected double ref addressability check
deelawn Sep 21, 2024
2fc5c88
fixed star expression addressability
deelawn Sep 23, 2024
eddcd54
added comment
deelawn Sep 25, 2024
b3d9f9a
Update gnovm/pkg/gnolang/nodes.go
deelawn Oct 7, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions gnovm/pkg/gnolang/addressability.go
thehowl marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package gnolang

type Addressability int

const (
AddressabilityNotApplicable Addressability = iota
AddressabilitySatisfied
AddressabilityUnsatisfied
)
195 changes: 164 additions & 31 deletions gnovm/pkg/gnolang/nodes.go
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,7 @@
type Expr interface {
Node
assertExpr()
addressability() Addressability
thehowl marked this conversation as resolved.
Show resolved Hide resolved
}

type Exprs []Expr
Expand Down Expand Up @@ -374,6 +375,10 @@
Name
}

func (x *NameExpr) addressability() Addressability {
thehowl marked this conversation as resolved.
Show resolved Hide resolved
return AddressabilitySatisfied
}

type NameExprs []NameExpr

type BasicLitExpr struct {
Expand All @@ -385,33 +390,65 @@
Value string
}

func (x *BasicLitExpr) addressability() Addressability {
return AddressabilityUnsatisfied

Check warning on line 394 in gnovm/pkg/gnolang/nodes.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/nodes.go#L393-L394

Added lines #L393 - L394 were not covered by tests
}

type BinaryExpr struct { // (Left Op Right)
Attributes
Left Expr // left operand
Op Word // operator
Right Expr // right operand
}

func (x *BinaryExpr) addressability() Addressability {
return AddressabilityUnsatisfied

Check warning on line 405 in gnovm/pkg/gnolang/nodes.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/nodes.go#L404-L405

Added lines #L404 - L405 were not covered by tests
}

type CallExpr struct { // Func(Args<Varg?...>)
Attributes
Func Expr // function expression
Args Exprs // function arguments, if any.
Varg bool // if true, final arg is variadic.
NumArgs int // len(Args) or len(Args[0].Results)
Func Expr // function expression
Args Exprs // function arguments, if any.
Varg bool // if true, final arg is variadic.
NumArgs int // len(Args) or len(Args[0].Results)
Addressability Addressability
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the cases like this where the addressability is assigned during preprocess, what do you think about moving Addressability as an attribute?

The reason why I say this is that we currently have a bunch of information which is not actually persisted, but recovered by using Preprocess. The Expr types here contain in their struct only information which comes directly out of the AST representation; not from preprocessing. I'm not entirely sure of why it's designed this way; but I think for consistency we should maintain the split between "preprocessed information" in Attributes, and AST information in the struct themselves.

If addressability is an attribute, I also wonder if there's a way to remove the specific type and turn this into Addressable() bool instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think there is a good way to make an Addressable() bool method due to reasons I outlined in responses to other comments -- there are one or more cases where addressability is not applicable so the preprocessor should handle execution as usual. That being said, while many other attributes apply only exist some of the time, the Addressability attribute exists for all addressable expressions -- everywhere it appears. For ease of access, I'd prefer not to make this an attribute.

}

func (x *CallExpr) addressability() Addressability {
return x.Addressability
}

type IndexExpr struct { // X[Index]
Attributes
X Expr // expression
Index Expr // index expression
HasOK bool // if true, is form: `value, ok := <X>[<Key>]
X Expr // expression
Index Expr // index expression
HasOK bool // if true, is form: `value, ok := <X>[<Key>]
Addressability Addressability
}

func (x *IndexExpr) addressability() Addressability {
// In this case NotApplicable means that it wasn't set, the default value.
if x.Addressability == AddressabilityNotApplicable {
thehowl marked this conversation as resolved.
Show resolved Hide resolved
return x.X.addressability()
}

return x.Addressability
}

type SelectorExpr struct { // X.Sel
Attributes
X Expr // expression
Path ValuePath // set by preprocessor.
Sel Name // field selector
X Expr // expression
Path ValuePath // set by preprocessor.
Sel Name // field selector
IsAddressable bool // true if X is a pointer
}

func (x *SelectorExpr) addressability() Addressability {
if x.IsAddressable || x.X.addressability() == AddressabilitySatisfied {
return AddressabilitySatisfied
}

return AddressabilityUnsatisfied
}

type SliceExpr struct { // X[Low:High:Max]
Expand All @@ -422,6 +459,10 @@
Max Expr // maximum capacity of slice; or nil; added in Go 1.2
}

func (x *SliceExpr) addressability() Addressability {
return AddressabilityUnsatisfied
}

// A StarExpr node represents an expression of the form
// "*" Expression. Semantically it could be a unary "*"
// expression, or a pointer type.
Expand All @@ -430,16 +471,33 @@
X Expr // operand
}

func (x *StarExpr) addressability() Addressability {
return x.X.addressability()

Check warning on line 475 in gnovm/pkg/gnolang/nodes.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/nodes.go#L474-L475

Added lines #L474 - L475 were not covered by tests
thehowl marked this conversation as resolved.
Show resolved Hide resolved
}

type RefExpr struct { // &X
Attributes
X Expr // operand
}

func (x *RefExpr) addressability() Addressability {
return x.X.addressability()

Check warning on line 484 in gnovm/pkg/gnolang/nodes.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/nodes.go#L483-L484

Added lines #L483 - L484 were not covered by tests
}

type TypeAssertExpr struct { // X.(Type)
Attributes
X Expr // expression.
Type Expr // asserted type, never nil.
HasOK bool // if true, is form: `_, ok := <X>.(<Type>)`.
X Expr // expression.
Type Expr // asserted type, never nil.
HasOK bool // if true, is form: `_, ok := <X>.(<Type>)`.
IsAddressable bool
}

func (x *TypeAssertExpr) addressability() Addressability {
if x.IsAddressable {
return AddressabilitySatisfied

Check warning on line 497 in gnovm/pkg/gnolang/nodes.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/nodes.go#L497

Added line #L497 was not covered by tests
}

return AddressabilityUnsatisfied
}

// A UnaryExpr node represents a unary expression. Unary
Expand All @@ -452,12 +510,25 @@
Op Word // operator
}

func (x *UnaryExpr) addressability() Addressability {
return x.X.addressability()

Check warning on line 514 in gnovm/pkg/gnolang/nodes.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/nodes.go#L513-L514

Added lines #L513 - L514 were not covered by tests
}

// MyType{<key>:<value>} struct, array, slice, and map
// expressions.
type CompositeLitExpr struct {
Attributes
Type Expr // literal type; or nil
Elts KeyValueExprs // list of struct fields; if any
Type Expr // literal type; or nil
Elts KeyValueExprs // list of struct fields; if any
IsAddressable bool
}

func (x *CompositeLitExpr) addressability() Addressability {
if x.IsAddressable {
return AddressabilitySatisfied
}

return AddressabilityUnsatisfied
}

// Returns true if any elements are keyed.
Expand Down Expand Up @@ -490,6 +561,10 @@
Value Expr // never nil
}

func (x *KeyValueExpr) addressability() Addressability {
return AddressabilityNotApplicable

Check warning on line 565 in gnovm/pkg/gnolang/nodes.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/nodes.go#L564-L565

Added lines #L564 - L565 were not covered by tests
}

type KeyValueExprs []KeyValueExpr

// A FuncLitExpr node represents a function literal. Here one
Expand All @@ -502,6 +577,10 @@
Body // function body
}

func (x *FuncLitExpr) addressability() Addressability {
return AddressabilityUnsatisfied
}

// The preprocessor replaces const expressions
// with *ConstExpr nodes.
type ConstExpr struct {
Expand All @@ -510,6 +589,10 @@
TypedValue
}

func (x *ConstExpr) addressability() Addressability {
return AddressabilityUnsatisfied
}

// ----------------------------------------
// Type(Expressions)
//
Expand Down Expand Up @@ -574,6 +657,10 @@
Tag Expr
}

func (x *FieldTypeExpr) addressability() Addressability {
return AddressabilityNotApplicable

Check warning on line 661 in gnovm/pkg/gnolang/nodes.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/nodes.go#L660-L661

Added lines #L660 - L661 were not covered by tests
}

type FieldTypeExprs []FieldTypeExpr

func (ftxz FieldTypeExprs) IsNamed() bool {
Expand All @@ -598,18 +685,30 @@
Elt Expr // element type
}

func (x *ArrayTypeExpr) addressability() Addressability {
return AddressabilityNotApplicable

Check warning on line 689 in gnovm/pkg/gnolang/nodes.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/nodes.go#L688-L689

Added lines #L688 - L689 were not covered by tests
}

type SliceTypeExpr struct {
Attributes
Elt Expr // element type
Vrd bool // variadic arg expression
}

func (x *SliceTypeExpr) addressability() Addressability {
return AddressabilityNotApplicable

Check warning on line 699 in gnovm/pkg/gnolang/nodes.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/nodes.go#L698-L699

Added lines #L698 - L699 were not covered by tests
}

type InterfaceTypeExpr struct {
Attributes
Methods FieldTypeExprs // list of methods
Generic Name // for uverse generics
}

func (x *InterfaceTypeExpr) addressability() Addressability {
return AddressabilityNotApplicable

Check warning on line 709 in gnovm/pkg/gnolang/nodes.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/nodes.go#L708-L709

Added lines #L708 - L709 were not covered by tests
}

type ChanDir int

const (
Expand All @@ -627,36 +726,60 @@
Value Expr // value type
}

func (x *ChanTypeExpr) addressability() Addressability {
return AddressabilityNotApplicable

Check warning on line 730 in gnovm/pkg/gnolang/nodes.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/nodes.go#L729-L730

Added lines #L729 - L730 were not covered by tests
}

type FuncTypeExpr struct {
Attributes
Params FieldTypeExprs // (incoming) parameters, if any.
Results FieldTypeExprs // (outgoing) results, if any.
}

func (x *FuncTypeExpr) addressability() Addressability {
return AddressabilityNotApplicable

Check warning on line 740 in gnovm/pkg/gnolang/nodes.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/nodes.go#L739-L740

Added lines #L739 - L740 were not covered by tests
thehowl marked this conversation as resolved.
Show resolved Hide resolved
}

type MapTypeExpr struct {
Attributes
Key Expr // const
Value Expr // value type
}

func (x *MapTypeExpr) addressability() Addressability {
return AddressabilityNotApplicable

Check warning on line 750 in gnovm/pkg/gnolang/nodes.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/nodes.go#L749-L750

Added lines #L749 - L750 were not covered by tests
}

type StructTypeExpr struct {
Attributes
Fields FieldTypeExprs // list of field declarations
}

func (x *StructTypeExpr) addressability() Addressability {
return AddressabilityNotApplicable

Check warning on line 759 in gnovm/pkg/gnolang/nodes.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/nodes.go#L758-L759

Added lines #L758 - L759 were not covered by tests
}

// Like ConstExpr but for types.
type constTypeExpr struct {
Attributes
Source Expr
Type Type
}

func (x *constTypeExpr) addressability() Addressability {
return AddressabilityNotApplicable

Check warning on line 770 in gnovm/pkg/gnolang/nodes.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/nodes.go#L769-L770

Added lines #L769 - L770 were not covered by tests
}

// Only used for native func arguments
type MaybeNativeTypeExpr struct {
Attributes
Type Expr
}

func (x *MaybeNativeTypeExpr) addressability() Addressability {
return AddressabilityNotApplicable

Check warning on line 780 in gnovm/pkg/gnolang/nodes.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/nodes.go#L779-L780

Added lines #L779 - L780 were not covered by tests
}

// ----------------------------------------
// Stmt
//
Expand Down Expand Up @@ -1484,6 +1607,7 @@
GetParentNode(Store) BlockNode
GetPathForName(Store, Name) ValuePath
GetIsConst(Store, Name) bool
GetIsConstAt(Store, ValuePath) bool
GetLocalIndex(Name) (uint16, bool)
GetValueRef(Store, Name, bool) *TypedValue
GetStaticTypeOf(Store, Name) Type
Expand Down Expand Up @@ -1671,6 +1795,30 @@
}
}

func (sb *StaticBlock) getAt(store Store, path ValuePath) *StaticBlock {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
func (sb *StaticBlock) getAt(store Store, path ValuePath) *StaticBlock {
func (sb *StaticBlock) GetBlockNodeForPath(store Store, path ValuePath) *StaticBlock {

if debug {
if path.Type != VPBlock {
panic("should not happen")

Check warning on line 1801 in gnovm/pkg/gnolang/nodes.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/nodes.go#L1800-L1801

Added lines #L1800 - L1801 were not covered by tests
deelawn marked this conversation as resolved.
Show resolved Hide resolved
}
if path.Depth == 0 {
panic("should not happen")

Check warning on line 1804 in gnovm/pkg/gnolang/nodes.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/nodes.go#L1803-L1804

Added lines #L1803 - L1804 were not covered by tests
}
}

for {
if path.Depth == 1 {
return sb
} else {
sb = sb.GetParentNode(store).GetStaticBlock()
path.Depth -= 1
}
}
}

func (sb *StaticBlock) GetIsConstAt(store Store, path ValuePath) bool {
return sb.getAt(store, path).getLocalIsConst(path.Name)
}

// Returns true iff n is a local const defined name.
func (sb *StaticBlock) getLocalIsConst(n Name) bool {
for _, name := range sb.Consts {
Expand Down Expand Up @@ -1706,22 +1854,7 @@

// Implements BlockNode.
func (sb *StaticBlock) GetStaticTypeOfAt(store Store, path ValuePath) Type {
if debug {
if path.Type != VPBlock {
panic("should not happen")
}
if path.Depth == 0 {
panic("should not happen")
}
}
for {
if path.Depth == 1 {
return sb.Types[path.Index]
} else {
sb = sb.GetParentNode(store).GetStaticBlock()
path.Depth -= 1
}
}
return sb.getAt(store, path).Types[path.Index]
}

// Implements BlockNode.
Expand Down
Loading
Loading