Skip to content

Commit

Permalink
refactor(gnolang): handle duplicate method decls using TryDefineMethod (
Browse files Browse the repository at this point in the history
#1459)

A (IMHO) better method of handling this. Related discussion:
#859 (comment)

Avoids creating a panic/recover in Preprocess by handling the
"redeclaration" case directly when it happens.

cc/ @jaekwon to confirm whether he agrees it's a good solution.

(suggestion: review with whitespace hidden)
  • Loading branch information
thehowl authored Jan 8, 2024
1 parent 6307e83 commit c6097cd
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 43 deletions.
22 changes: 8 additions & 14 deletions gnovm/pkg/gnolang/preprocess.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,18 +118,6 @@ var preprocessing int
// - Assigns BlockValuePath to NameExprs.
// - TODO document what it does.
func Preprocess(store Store, ctx BlockNode, n Node) Node {
// When panic, revert any package updates.
defer func() {
// Revert all new values.
// this is needed to revert top level
// function redeclarations.
if r := recover(); r != nil {
pkg := packageOf(ctx)
pkg.StaticBlock.revertToOld()
panic(r)
}
}()

// Increment preprocessing counter while preprocessing.
{
preprocessing += 1
Expand Down Expand Up @@ -2933,7 +2921,7 @@ func predefineNow2(store Store, last BlockNode, d Decl, m map[Name]struct{}) (De
} else {
dt = rt.(*DeclaredType)
}
dt.DefineMethod(&FuncValue{
if !dt.TryDefineMethod(&FuncValue{
Type: ft,
IsMethod: true,
Source: cd,
Expand All @@ -2943,7 +2931,13 @@ func predefineNow2(store Store, last BlockNode, d Decl, m map[Name]struct{}) (De
PkgPath: pkg.PkgPath,
body: cd.Body,
nativeBody: nil,
})
}) {
// Revert to old function declarations in the package we're preprocessing.
pkg := packageOf(last)
pkg.StaticBlock.revertToOld()
panic(fmt.Sprintf("redeclaration of method %s.%s",
dt.Name, cd.Name))
}
} else {
ftv := pkg.GetValueRef(store, cd.Name)
ft := ftv.T.(*FuncType)
Expand Down
69 changes: 40 additions & 29 deletions gnovm/pkg/gnolang/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -1451,50 +1451,61 @@ func (dt *DeclaredType) GetPkgPath() string {
}

func (dt *DeclaredType) DefineMethod(fv *FuncValue) {
if !dt.TryDefineMethod(fv) {
panic(fmt.Sprintf("redeclaration of method %s.%s",
dt.Name, fv.Name))
}
}

// TryDefineMethod attempts to define the method fv on type dt.
// It returns false if this does not succeeds, as a result of a re-declaration.
func (dt *DeclaredType) TryDefineMethod(fv *FuncValue) bool {
name := fv.Name

// Handle redeclarations.
for i, tv := range dt.Methods {
ofv := tv.V.(*FuncValue)
if ofv.Name == name {
// Do not allow redeclaring (override) a method.
// In the future we may allow this, just like we
// allow package-level function overrides.

// Special case: if the type and location are the same,
// ignore and do not redefine.
// This is due to PreprocessAllFilesAndSaveBlocknodes,
// and because the preprocessor fills some of the
// method's FuncValue. Since the method was already
// filled in prior to PreprocessAllFilesAndSaveBlocks,
// there is no need to re-set it.
// Keep this or move this check outside.
if fv.Type.TypeID() == ofv.Type.TypeID() &&
fv.Source.GetLocation() == ofv.Source.GetLocation() {
return
}
if ofv.Name != name {
continue
}

// Special case: allow defining a native body.
if fv.Type.TypeID() == ofv.Type.TypeID() &&
!ofv.IsNative() && fv.IsNative() {
dt.Methods[i] = TypedValue{
T: fv.Type, // keep old type.
V: fv,
}
return
}
// Do not allow redeclaring (override) a method.
// In the future we may allow this, just like we
// allow package-level function overrides.

// Special case: if the type and location are the same,
// ignore and do not redefine.
// This is due to PreprocessAllFilesAndSaveBlocknodes,
// and because the preprocessor fills some of the
// method's FuncValue. Since the method was already
// filled in prior to PreprocessAllFilesAndSaveBlocks,
// there is no need to re-set it.
// Keep this or move this check outside.
if fv.Type.TypeID() == ofv.Type.TypeID() &&
fv.Source.GetLocation() == ofv.Source.GetLocation() {
return true
}

// Otherwise panic.
panic(fmt.Sprintf("redeclaration of method %s.%s",
dt.Name, name))
// Special case: allow defining a native body.
if fv.Type.TypeID() == ofv.Type.TypeID() &&
!ofv.IsNative() && fv.IsNative() {
dt.Methods[i] = TypedValue{
T: fv.Type, // keep old type.
V: fv,
}
return true
}

// Otherwise fail and return false.
return false
}

// If not redeclaring, just append.
dt.Methods = append(dt.Methods, TypedValue{
T: fv.Type,
V: fv,
})
return true
}

func (dt *DeclaredType) GetPathForName(n Name) ValuePath {
Expand Down

0 comments on commit c6097cd

Please sign in to comment.