From 75c04e25cd9ccc811d6893fc0c0c02df889cad66 Mon Sep 17 00:00:00 2001 From: xhd2015 Date: Mon, 8 Apr 2024 14:30:39 +0800 Subject: [PATCH] fix patching const test --- .github/workflows/go.yml | 14 +-- CONTRIBUTING.md | 8 +- cmd/xgo/patch_compiler.go | 2 + cmd/xgo/patch_compiler_ast_type_check.go | 76 ++++++++++++- cmd/xgo/version.go | 4 +- patch/syntax/rewrite.go | 23 ++++ patch/syntax/syntax.go | 18 +-- patch/syntax/vars.go | 105 +++++++++--------- runtime/core/version.go | 4 +- runtime/test/debug/debug_test.go | 37 +++--- runtime/test/debug/sub/sub.go | 10 ++ .../test/patch_const/patch_const_computed.go | 37 ++++++ runtime/test/patch_const/patch_const_test.go | 60 ++++++++++ runtime/test/patch_const/sub/sub.go | 2 + 14 files changed, 302 insertions(+), 98 deletions(-) create mode 100644 runtime/test/debug/sub/sub.go create mode 100644 runtime/test/patch_const/patch_const_computed.go diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index 42979db4..95f83642 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -22,6 +22,12 @@ jobs: go-version: '1.20' cache: false + - name: Check spelling of files + uses: crate-ci/typos@master + continue-on-error: false + with: + files: ./ + - name: Build run: go build -v ./cmd/... @@ -41,10 +47,4 @@ jobs: run: ~/.xgo/bin/xgo revision - name: Check Go Version - run: ~/.xgo/bin/xgo exec go version - - - name: Check spelling of files - uses: crate-ci/typos@master - continue-on-error: false - with: - files: ./ \ No newline at end of file + run: ~/.xgo/bin/xgo exec go version \ No newline at end of file diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index f30d4726..c50b6a1a 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -5,7 +5,7 @@ Thanks for helping, this document helps you get started. First, clone this repository: ```sh git clone https://github.com/xhd2015/xgo -cd cgo +cd xgo ``` Then, setup git hooks: @@ -19,12 +19,14 @@ chmod +x .git/hooks/post-commit All are set, now start to development, try: ```sh # help -go run ./cmd/xgo help +go -tags dev run ./cmd/xgo help # run Hello world test -go test -run TestHelloWorld -v ./test +go -tags dev test -run TestHelloWorld -v ./test ``` +NOTE: when developing, always add `-tags dev` to tell go that we building with dev mode. + If you want to check instrumented GOROOT, run: ```sh go run ./script/setup-dev diff --git a/cmd/xgo/patch_compiler.go b/cmd/xgo/patch_compiler.go index 00baf12d..687ad128 100644 --- a/cmd/xgo/patch_compiler.go +++ b/cmd/xgo/patch_compiler.go @@ -44,6 +44,8 @@ var compilerFiles = []_FilePath{ type2AssignmentsPatch.FilePath, syntaxWalkPatch.FilePath, noderWriterPatch.FilePath, + noderExprPatch.FilePath, + syntaxPrinterPatch.FilePath, syntaxExtra, } diff --git a/cmd/xgo/patch_compiler_ast_type_check.go b/cmd/xgo/patch_compiler_ast_type_check.go index 04fb9009..1c3bdb79 100644 --- a/cmd/xgo/patch_compiler_ast_type_check.go +++ b/cmd/xgo/patch_compiler_ast_type_check.go @@ -9,10 +9,15 @@ import ( const convertXY = ` if xgoConv, ok := x.expr.(*syntax.XgoSimpleConvert); ok { var isConst bool - switch y.expr.(type) { - case *syntax.XgoSimpleConvert,*syntax.BasicLit: - isConst=true + if isUntyped(y.typ) { + isConst = true + } else { + switch y.expr.(type) { + case *syntax.XgoSimpleConvert, *syntax.BasicLit: + isConst = true + } } + if !isConst{ t := y.typ @@ -35,9 +40,13 @@ if xgoConv, ok := x.expr.(*syntax.XgoSimpleConvert); ok { } }else if xgoConv,ok := y.expr.(*syntax.XgoSimpleConvert);ok { var isConst bool - switch x.expr.(type) { - case *syntax.XgoSimpleConvert,*syntax.BasicLit: - isConst=true + if isUntyped(x.typ) { + isConst = true + } else { + switch x.expr.(type) { + case *syntax.XgoSimpleConvert, *syntax.BasicLit: + isConst = true + } } if !isConst{ t := x.typ @@ -228,6 +237,51 @@ var noderWriterPatch = &FilePatch{ }, }, } + +var noderExprPatch = &FilePatch{ + FilePath: _FilePath{"src", "cmd", "compile", "internal", "noder", "expr.go"}, + Patches: []*Patch{ + { + Mark: "noder_expr_const_expr_op_xgo_simple_convert", + InsertIndex: 1, + InsertBefore: true, + Anchors: []string{ + `func constExprOp(expr syntax.Expr) ir.Op {`, + `case *syntax.BasicLit:`, + }, + Content: ` + case *syntax.XgoSimpleConvert: + return constExprOp(expr.X) + `, + CheckGoVersion: func(goVersion *goinfo.GoVersion) bool { + return goVersion.Major == 1 && goVersion.Minor <= 21 + }, + }, + }, +} + +var syntaxPrinterPatch = &FilePatch{ + FilePath: _FilePath{"src", "cmd", "compile", "internal", "syntax", "printer.go"}, + Patches: []*Patch{ + { + Mark: "noder_syntax_print_xgo_simple_convert", + InsertIndex: 1, + InsertBefore: true, + Anchors: []string{ + `func (p *printer) printRawNode(n Node) {`, + `case *BasicLit:`, + }, + Content: ` + case *XgoSimpleConvert: + p.printRawNode(n.X) + `, + CheckGoVersion: func(goVersion *goinfo.GoVersion) bool { + return goVersion.Major == 1 && goVersion.Minor <= 21 + }, + }, + }, +} + var syntaxExtra = _FilePath{"src", "cmd", "compile", "internal", "syntax", "xgo_extra.go"} const syntaxExtraPatch = ` @@ -268,5 +322,15 @@ func patchCompilerAstTypeCheck(goroot string, goVersion *goinfo.GoVersion) error if err != nil { return err } + if goVersion.Major == 1 && goVersion.Minor <= 21 { + err = noderExprPatch.Apply(goroot, goVersion) + if err != nil { + return err + } + err = syntaxPrinterPatch.Apply(goroot, goVersion) + if err != nil { + return err + } + } return nil } diff --git a/cmd/xgo/version.go b/cmd/xgo/version.go index 027d737a..6b52bef3 100644 --- a/cmd/xgo/version.go +++ b/cmd/xgo/version.go @@ -3,8 +3,8 @@ package main import "fmt" const VERSION = "1.0.19" -const REVISION = "fe2e2f38793f24bda0a8690eedba01828ef79916+1" -const NUMBER = 169 +const REVISION = "9f58dbf7230665a152fdc35eca356fb0f269a85d_DEV_2024-04-08T07:03:58Z" +const NUMBER = 170 func getRevision() string { revSuffix := "" diff --git a/patch/syntax/rewrite.go b/patch/syntax/rewrite.go index e450b7b2..d2ec0a21 100644 --- a/patch/syntax/rewrite.go +++ b/patch/syntax/rewrite.go @@ -252,7 +252,30 @@ func fillNames(pos syntax.Pos, recv *syntax.Field, funcType *syntax.FuncType, pr fillFieldNames(pos, funcType.ParamList, preset, "_a") fillFieldNames(pos, funcType.ResultList, preset, "_r") } +func getFuncDeclNamesNoBlank(recv *syntax.Field, funcType *syntax.FuncType) []string { + var names []string + recvName := getFieldName(recv) + if !isBlankName(recvName) { + names = append(names, recvName) + } + paramNames := getFieldNames(funcType.ParamList) + for _, name := range paramNames { + if !isBlankName(name) { + names = append(names, name) + } + } + resultNames := getFieldNames(funcType.ResultList) + for _, name := range resultNames { + if !isBlankName(name) { + names = append(names, name) + } + } + return names +} +func isBlankName(name string) bool { + return name == "" || name == "_" +} func getRefSlice(pos syntax.Pos, fields []*syntax.Field) []syntax.Expr { return doGetRefAddrSlice(pos, fields, false) } diff --git a/patch/syntax/syntax.go b/patch/syntax/syntax.go index f8ad43de..91de11a2 100644 --- a/patch/syntax/syntax.go +++ b/patch/syntax/syntax.go @@ -765,17 +765,19 @@ func generateRegHelperCode(pkgName string) string { return strings.Join(autoGenStmts, "\n") } -func getFieldNames(x []*syntax.Field) []string { - names := make([]string, 0, len(x)) - for _, p := range x { - var name string - if p.Name != nil { - name = p.Name.Value - } - names = append(names, name) +func getFieldNames(fields []*syntax.Field) []string { + names := make([]string, 0, len(fields)) + for _, field := range fields { + names = append(names, getFieldName(field)) } return names } +func getFieldName(f *syntax.Field) string { + if f == nil || f.Name == nil { + return "" + } + return f.Name.Value +} func quoteNamesExpr(names []string) string { if len(names) == 0 { diff --git a/patch/syntax/vars.go b/patch/syntax/vars.go index 01f57029..96852a03 100644 --- a/patch/syntax/vars.go +++ b/patch/syntax/vars.go @@ -84,7 +84,14 @@ func trapVariables(pkgPath string, fileList []*syntax.File, funcDelcs []*DeclInf if fnDecl.Body == nil { continue } - ctx := &BlockContext{} + // each function is a + ctx := &BlockContext{ + Names: make(map[string]bool), + } + argNames := getFuncDeclNamesNoBlank(fnDecl.Recv, fnDecl.Type) + for _, argName := range argNames { + ctx.Names[argName] = true + } fnDecl.Body = ctx.traverseBlockStmt(fnDecl.Body, names, imports) } } @@ -119,12 +126,10 @@ func getImports(file *syntax.File) map[string]string { return imports } +// a BlockContext provides a point where stmts can be prepended or inserted type BlockContext struct { Parent *BlockContext Block *syntax.BlockStmt - Index int - - Children []*BlockContext Names map[string]bool @@ -141,10 +146,16 @@ type BlockContext struct { ConstInfo map[syntax.Node]*ConstInfo // to be inserted - InsertList []syntax.Stmt + ChildrenInsertList [][]syntax.Stmt TrapNames []*NameAndDecl } + +func (c *BlockContext) PrependStmtBeforeLastChild(stmt []syntax.Stmt) { + n := len(c.ChildrenInsertList) + c.ChildrenInsertList[n-1] = append(c.ChildrenInsertList[n-1], stmt...) +} + type ConstInfo struct { Type string } @@ -311,19 +322,16 @@ func (ctx *BlockContext) traverseBlockStmt(node *syntax.BlockStmt, globaleNames if node == nil { return nil } + subCtx := &BlockContext{ + Parent: ctx, + } n := len(node.List) - base := len(ctx.Children) for i := 0; i < n; i++ { - subCtx := &BlockContext{ - Parent: ctx, - Block: node, - Index: i, - } - ctx.Children = append(ctx.Children, subCtx) + subCtx.ChildrenInsertList = append(subCtx.ChildrenInsertList, nil) node.List[i] = subCtx.traverseStmt(node.List[i], globaleNames, imports) } for i := n - 1; i >= 0; i-- { - node.List = insertBefore(node.List, i, ctx.Children[i+base].InsertList) + node.List = insertBefore(node.List, i, subCtx.ChildrenInsertList[i]) } return node } @@ -376,12 +384,16 @@ func (ctx *BlockContext) traverseExpr(node syntax.Expr, globaleNames map[string] case *syntax.KeyValueExpr: node.Value = ctx.traverseExpr(node.Value, globaleNames, imports) case *syntax.FuncLit: - subCtx := &BlockContext{ + // add names of function declares + funcCtx := &BlockContext{ Parent: ctx, + Names: make(map[string]bool), + } + argNames := getFuncDeclNamesNoBlank(nil, node.Type) + for _, argName := range argNames { + funcCtx.Names[argName] = true } - // TODO: add names of types - ctx.Children = append(ctx.Children, subCtx) - node.Body = subCtx.traverseBlockStmt(node.Body, globaleNames, imports) + node.Body = funcCtx.traverseBlockStmt(node.Body, globaleNames, imports) return node case *syntax.ParenExpr: node.X = ctx.traverseExpr(node.X, globaleNames, imports) @@ -451,20 +463,20 @@ func (ctx *BlockContext) traverseExpr(node syntax.Expr, globaleNames map[string] if node.X != nil && node.Y != nil { xConst := ctx.ConstInfo[node.X] yConst := ctx.ConstInfo[node.Y] - if xConst != nil && yConst != nil { - newNode := &syntax.XgoSimpleConvert{ - X: &syntax.CallExpr{ - Fun: syntax.NewName(node.Pos(), xConst.Type), - ArgList: []syntax.Expr{node}, - }, - } + isXgoConv := func(node syntax.Expr) bool { + _, ok := node.(*syntax.XgoSimpleConvert) + return ok + } + // if both are constant,skip wrapping + if xConst != nil && yConst != nil && (isXgoConv(node.X) || isXgoConv(node.Y)) { + newNode := newConv(node, xConst.Type) ctx.recordConstType(newNode, xConst.Type) return newNode } } else if node.Y == nil && (node.Op == syntax.Add || node.Op == syntax.Sub) { if xgoConv, ok := node.X.(*syntax.XgoSimpleConvert); ok { constType := getConstType(xgoConv) - newNode := createConv(node, constType) + newNode := newConv(node, constType) ctx.recordConstType(newNode, constType) return newNode } @@ -512,7 +524,7 @@ func getConstType(xgoConv *syntax.XgoSimpleConvert) string { return xgoConv.X.(*syntax.CallExpr).Fun.(*syntax.Name).Value } -func createConv(node syntax.Expr, constType string) *syntax.XgoSimpleConvert { +func newConv(node syntax.Expr, constType string) *syntax.XgoSimpleConvert { return &syntax.XgoSimpleConvert{ X: &syntax.CallExpr{ Fun: syntax.NewName(node.Pos(), constType), @@ -593,6 +605,9 @@ func (c *BlockContext) trapValueNode(node *syntax.Name, globaleNames map[string] return node } untypedConstType = getConstDeclValueType(decl.ConstDecl.Values) + if untypedConstType == "" { + return node + } var ok bool explicitType, ok = c.isConstOKToTrap(node) if !ok { @@ -620,7 +635,7 @@ func (c *BlockContext) trapValueNode(node *syntax.Name, globaleNames map[string] }, preStmts...) } - c.InsertList = append(c.InsertList, preStmts...) + c.PrependStmtBeforeLastChild(preStmts) newName := syntax.NewName(node.Pos(), tmpVarName) if explicitType != nil { return &syntax.CallExpr{ @@ -631,18 +646,8 @@ func (c *BlockContext) trapValueNode(node *syntax.Name, globaleNames map[string] } } if untypedConstType != "" { - newNode := &syntax.XgoSimpleConvert{ - X: &syntax.CallExpr{ - Fun: syntax.NewName(node.Pos(), untypedConstType), - ArgList: []syntax.Expr{ - newName, - }, - }, - } - if c.ConstInfo == nil { - c.ConstInfo = make(map[syntax.Node]*ConstInfo, 1) - } - c.ConstInfo[node] = &ConstInfo{Type: untypedConstType} + newNode := newConv(newName, untypedConstType) + c.recordConstType(newNode, untypedConstType) c.ConstInfo[newNode] = &ConstInfo{Type: untypedConstType} return newNode } @@ -679,6 +684,9 @@ func (ctx *BlockContext) trapSelector(node syntax.Expr, sel *syntax.SelectorExpr return nil, true } untypedConstType = constInfo.Type + if untypedConstType == "" { + return nil, true + } var ok bool explicitType, ok = ctx.isConstOKToTrap(node) if !ok { @@ -698,7 +706,7 @@ func (ctx *BlockContext) trapSelector(node syntax.Expr, sel *syntax.SelectorExpr return nil, true } preStmts, _, tmpVarName := trapVar(node, newStringLit(pkgPath), sel.Sel.Value, takeAddr) - ctx.InsertList = append(ctx.InsertList, preStmts...) + ctx.PrependStmtBeforeLastChild(preStmts) newName := syntax.NewName(node.Pos(), tmpVarName) if explicitType != nil { return &syntax.CallExpr{ @@ -709,19 +717,8 @@ func (ctx *BlockContext) trapSelector(node syntax.Expr, sel *syntax.SelectorExpr }, true } if untypedConstType != "" { - newNode := &syntax.XgoSimpleConvert{ - X: &syntax.CallExpr{ - Fun: syntax.NewName(node.Pos(), untypedConstType), - ArgList: []syntax.Expr{ - newName, - }, - }, - } - if ctx.ConstInfo == nil { - ctx.ConstInfo = make(map[syntax.Node]*ConstInfo, 1) - } - ctx.ConstInfo[sel] = &ConstInfo{Type: untypedConstType} - ctx.ConstInfo[node] = &ConstInfo{Type: untypedConstType} + newNode := newConv(newName, untypedConstType) + ctx.recordConstType(newNode, untypedConstType) return newNode, true } return newName, true @@ -802,7 +799,7 @@ func (c *BlockContext) trapAddrNode(node *syntax.Operation, nameNode *syntax.Nam return node } preStmts, _, tmpVarName := trapVar(node, syntax.NewName(nameNode.Pos(), XgoLocalPkgName), name, true) - c.InsertList = append(c.InsertList, preStmts...) + c.PrependStmtBeforeLastChild(preStmts) return syntax.NewName(node.Pos(), tmpVarName) } diff --git a/runtime/core/version.go b/runtime/core/version.go index 763ee5eb..78f82398 100644 --- a/runtime/core/version.go +++ b/runtime/core/version.go @@ -7,8 +7,8 @@ import ( ) const VERSION = "1.0.19" -const REVISION = "fe2e2f38793f24bda0a8690eedba01828ef79916+1" -const NUMBER = 169 +const REVISION = "9f58dbf7230665a152fdc35eca356fb0f269a85d_DEV_2024-04-08T07:03:58Z" +const NUMBER = 170 // these fields will be filled by compiler const XGO_VERSION = "" diff --git a/runtime/test/debug/debug_test.go b/runtime/test/debug/debug_test.go index 6fc0b04f..468ba9f4 100644 --- a/runtime/test/debug/debug_test.go +++ b/runtime/test/debug/debug_test.go @@ -7,25 +7,30 @@ package debug import ( "testing" - - "github.com/xhd2015/xgo/runtime/mock" ) -const N = 50 -const M = 20 +const good = 2 +const reason = "test" func TestPatchConstOperationShouldCompileAndSkipMock(t *testing.T) { - // should have no effect - mock.PatchByName("github.com/xhd2015/xgo/runtime/test/debug", "N", func() int { - return 10 - }) - // because N is used inside an operation - // it's type is not yet determined, so - // should not rewrite it - var size int64 = M + N - t.Logf("size=%d", size) - // size := (N + 1) * unsafe.Sizeof(int(0)) - if size != 11 { - t.Fatalf("expect N not patched and size to be %d, actual: %d\n", 11, size) + reasons := getReasons("good") + if len(reasons) != 2 || reasons[0] != "ok" || reasons[1] != "good" { + t.Fatalf("bad reason: %v", reasons) + } + + getReasons2 := func(good string) (reason []string) { + reason = append(reason, "ok") + reason = append(reason, good) + return } + reasons2 := getReasons2("good") + if len(reasons2) != 2 || reasons2[0] != "ok" || reasons2[1] != "good" { + t.Fatalf("bad reason2: %v", reasons2) + } +} + +func getReasons(good string) (reason []string) { + reason = append(reason, "ok") + reason = append(reason, good) + return } diff --git a/runtime/test/debug/sub/sub.go b/runtime/test/debug/sub/sub.go new file mode 100644 index 00000000..e13aafbc --- /dev/null +++ b/runtime/test/debug/sub/sub.go @@ -0,0 +1,10 @@ +package sub + +const LabelPrefix = "label:" + +const Version = "v2" + +const ( + LabelPrefix2 = "label2:" + LabelPrefix3 = "label3:" +) diff --git a/runtime/test/patch_const/patch_const_computed.go b/runtime/test/patch_const/patch_const_computed.go new file mode 100644 index 00000000..b5a949b1 --- /dev/null +++ b/runtime/test/patch_const/patch_const_computed.go @@ -0,0 +1,37 @@ +//go:build go1.20 +// +build go1.20 + +package patch_const + +import "testing" + +const a = iota + +const ( + b0 = iota + b1 +) + +const c = 1<<2 + 3 + +func TestConstComputedShouldCompile(t *testing.T) { + var testA int64 = a + if testA != 0 { + t.Fatalf("expect testA to be %d, actual: %d", 0, testA) + } + + var testB0 int64 = b0 + if testB0 != 0 { + t.Fatalf("expect testB0 to be %d, actual: %d", 0, testB0) + } + + var testB1 int64 = b1 + if testB1 != 0 { + t.Fatalf("expect testB1 to be %d, actual: %d", 1, testB1) + } + + var testC int64 = c + if testC != 7 { + t.Fatalf("expect testC to be %d, actual: %d", 7, testC) + } +} diff --git a/runtime/test/patch_const/patch_const_test.go b/runtime/test/patch_const/patch_const_test.go index cec73740..7acdda7b 100644 --- a/runtime/test/patch_const/patch_const_test.go +++ b/runtime/test/patch_const/patch_const_test.go @@ -249,3 +249,63 @@ func TestPatchOtherPackageConstInTypeConvertArgShouldWork(t *testing.T) { t.Fatalf("expect a to be %d, actual: %d\n", 10, a) } } + +const x = "123" + +func TestPatchConstOverlappingNameShouldSkip(t *testing.T) { + x := make([]int, 0, 10) + x = append(x, 10) + if x[0] != 10 { + t.Fatalf("expect x[0] to be %d, actual: %d", 10, x[0]) + } +} + +func exampleSprintf(args ...interface{}) string { + return fmt.Sprintf("%v", args...) +} +func TestPatchLitPlusLitShouldCompile(t *testing.T) { + s := "should " + a := exampleSprintf(s + "compile") + b := exampleSprintf("should " + "compile") // this skips from wrapping + if a != b { + t.Fatalf("expect exampleSprintf result to be %q, actual: %q", b, a) + } +} + +type Label string + +func convertLabel(label Label) string { + return string(label) +} +func TestPatchOtherPkgConst(t *testing.T) { + label := convertLabel(sub.LabelPrefix + "v2") + if label != "label:v2" { + t.Fatalf("bad label: %s", label) + } +} + +const good = 2 +const reason = "test" + +func TestNameConflictWithArgShouldSkip(t *testing.T) { + reasons := getReasons("good") + if len(reasons) != 2 || reasons[0] != "ok" || reasons[1] != "good" { + t.Fatalf("bad reason: %v", reasons) + } + + getReasons2 := func(good string) (reason []string) { + reason = append(reason, "ok") + reason = append(reason, good) + return + } + reasons2 := getReasons2("good") + if len(reasons2) != 2 || reasons2[0] != "ok" || reasons2[1] != "good" { + t.Fatalf("bad reason2: %v", reasons2) + } +} + +func getReasons(good string) (reason []string) { + reason = append(reason, "ok") + reason = append(reason, good) + return +} diff --git a/runtime/test/patch_const/sub/sub.go b/runtime/test/patch_const/sub/sub.go index 8348c677..a5943240 100644 --- a/runtime/test/patch_const/sub/sub.go +++ b/runtime/test/patch_const/sub/sub.go @@ -1,3 +1,5 @@ package sub const N = 50 + +const LabelPrefix = "label:"