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

Support the incoming go1.23 #169

Closed
xhd2015 opened this issue May 30, 2024 · 6 comments
Closed

Support the incoming go1.23 #169

xhd2015 opened this issue May 30, 2024 · 6 comments
Labels
proposal Proposal new ideas
Milestone

Comments

@xhd2015
Copy link
Owner

xhd2015 commented May 30, 2024

Go 1.23 will arrive in August, should test with some RC versions.

@xhd2015 xhd2015 added this to the v1.1.0 milestone May 30, 2024
@xhd2015 xhd2015 added the proposal Proposal new ideas label May 30, 2024
@xhd2015
Copy link
Owner Author

xhd2015 commented Jul 11, 2024

After trying to support go1.23rc1, the first error was emitted:

Run GOROOT=$PWD/setup/go PATH=$PWD/setup/go/bin:$PATH go run ./script/run-test --reset-instrument --debug -v
TEST /home/runner/work/xgo/xgo/setup/go
resetting instrument
202[4](https://github.com/xhd2015/xgo/actions/runs/9892387182/job/27324927947?pr=249#step:4:5)-07-11 13:30:43 start: [/tmp/go-build109688[5](https://github.com/xhd2015/xgo/actions/runs/9892387182/job/27324927947?pr=249#step:4:6)578/b001/exe/xgo build --reset-instrument --with-goroot /home/runner/work/xgo/xgo/setup/go --build-compiler --log-debug=stdout]
2024-07-11 13:30:43 runtime.GOOS=linux
2024-07-11 13:30:43 runtime.GOARCH=amd[6](https://github.com/xhd2015/xgo/actions/runs/9892387182/job/27324927947?pr=249#step:4:7)4
2024-07-11 13:30:43 runtime.Version()=go1.23rc1
2024-07-11 13:30:43 runtime.GOROOT()=/home/runner/work/xgo/xgo/setup/go
2024-07-11 13:30:43 os exe suffix: 
2024-07-11 13:30:43 os force copy unsym: false
2024-0[7](https://github.com/xhd2015/xgo/actions/runs/9892387182/job/27324927947?pr=249#step:4:8)-11 13:30:43 effective GOROOT: /home/runner/work/xgo/xgo/setup/go
2024-07-11 13:30:43 go version: go1.23.0
2024-07-11 13:30:43 instrument dir: /home/runner/.xgo/go-instrument/go1.23.0_ho_ru_wo_xg_xg_se_go_c66f4dd7
2024-07-11 13:30:43 current core revision: 1.0.43 f1cf669[8](https://github.com/xhd2015/xgo/actions/runs/9892387182/job/27324927947?pr=249#step:4:9)521d5b43da06f012ac3ba5afb1308d27+1 BUILD_280, last core revision:  from file /home/runner/.xgo/go-instrument/go1.23.0_ho_ru_wo_xg_xg_se_go_c66f4dd7/xgo-revision.txt
2024-07-11 13:30:43 reset instrument /home/runner/.xgo/go-instrument/go1.23.0_ho_ru_wo_xg_xg_se_go_c66f4dd7
2024-07-11 13:30:43 sync goroot /home/runner/work/xgo/xgo/setup/go -> /home/runner/.xgo/go-instrument/go1.23.0_ho_ru_wo_xg_xg_se_go_c66f4dd7/go1.23.0
2024-07-11 13:30:43 patch compiler at: /home/runner/.xgo/go-instrument/go1.23.0_ho_ru_wo_xg_xg_se_go_c66f4dd7/go1.23.0
panic: sequence missing: [func newproc(fn systemstack(func() { newg := newproc1(fn, gp, pc)]

goroutine 1 [running]:
main.addContentAtIndex({0xc001334000, 0x33f30}, {0x7f4033, 0x1c}, {0x7f2a7d, 0x1a}, {0xc0000[9](https://github.com/xhd2015/xgo/actions/runs/9892387182/job/27324927947?pr=249#step:4:10)eff0, 0x3, 0x3}, 0x1, ...)
	/home/runner/work/xgo/xgo/cmd/xgo/edit.go:59 +0x1f6
main.patchRuntimeProc.func1({0xc001252000?, 0xc000f94000?})
	/home/runner/work/xgo/xgo/cmd/xgo/patch_runtime.go:161 +0x26d
main.editFile({0xc000e34070, 0x63}, 0xc00003f3a0)
	/home/runner/work/xgo/xgo/cmd/xgo/edit.go:18 +0x5d
main.patchRuntimeProc({0xc00012a0a0, 0x4f}, 0xc00011a000)
	/home/runner/work/xgo/xgo/cmd/xgo/patch_runtime.go:141 +0x178
main.patchRuntimeAndTesting({0xc00012a0a0, 0x4f}, 0x1?)
	/home/runner/work/xgo/xgo/cmd/xgo/patch_runtime.go:61 +0x1d
main.patchRuntimeAndCompiler({0x7ffdfa79bd[10](https://github.com/xhd2015/xgo/actions/runs/9892387182/job/27324927947?pr=249#step:4:11), 0x22}, {0xc00012a0a0, 0x4f}, {0x0, 0x0}, 0xc00011a000, 0x1, 0x1)
	/home/runner/work/xgo/xgo/cmd/xgo/patch.go:56 +0x77
main.handleBuild({0x7ffdfa79bce9, 0x5}, {0xc000024090?, 0x0?, 0x0?})
	/home/runner/work/xgo/xgo/cmd/xgo/main.go:335 +0x17c5
main.main()
	/home/runner/work/xgo/xgo/cmd/xgo/main.go:[11](https://github.com/xhd2015/xgo/actions/runs/9892387182/job/27324927947?pr=249#step:4:12)0 +0x5e5
exit status 2

@xhd2015
Copy link
Owner Author

xhd2015 commented Jul 16, 2024

Update:

Use git diff --no-index $GOROOT/src ~/.xgo/goroot/src > go.patch to generate diff patches.

And then use git apply < go.patch to apply the patch.

The problem with git diff is that it is not always safe to apply the patch because the base which the diff was generated from may have changed.

For example, if the diff is generated based on go1.22, and it is applied to go1.23. The patch may succeed, but it could cause semantic/logical error.

So before applying the patch, confirming whether the base file satisfies the incoming patch is important.

@xhd2015
Copy link
Owner Author

xhd2015 commented Jul 16, 2024

The patch contains 2 parts:

Patch {
    Assert  // asserts the target which will be operated satisfies some constraints
    Diff // the actual diff, specifying where to add, remove or replace some content
}

The most critical part is the diff, it contains instructions on how to edit the target file.

Current strategy is to text searching the target file, and if some sequence is found, a code snippet is inserted into a given position.

So, basically, the edit looks like:

Edit {
    Sequence
    Insertion | Deletion | Addition
}

The sequence is a simple list of plain strings, that's why it fails with go1.23.

For this upgrade, we can enhance the sequence to be a list of semantic tokens, possibly adding some contextual information, like:

{
    "comment": "add something after some place if some condition happen",
    "func": "some name",
    "children":[{
           "for_stmt":true
    }]
}

The edit can be nested. It represents an ast.Walk with some edit actions.

@xhd2015
Copy link
Owner Author

xhd2015 commented Jul 21, 2024

A natural thought would be to define the patch within the go source code context:

gc/main.go.patch

package syntax

import (
     "github.com/xhd2015/xgo/runtime"
)

func SomeFunc(){
     for _, v := range x {
         //add <id>
         runtime.Declare(...)
     }
}

The <id> is to identify this insertion so that the patch is idempotent.

If there are specific match condition for specific go version, use this notion //go:1.23.

After all patches applied to a single file, the imports will be deduplicated.

@xhd2015
Copy link
Owner Author

xhd2015 commented Jul 21, 2024

The patch applying process is a recursive process:

// srcCode, patchCode represents the head
// pointer of the AST tree for each respectively
func applyPatch(srcCode, patchCode AST) {
	srcHead := srcCode
	patchHead := patchCode

	for {
		if patchCode == nil {
			return
		}

		if srcCode == nil {
			panic("bad patch")
		}
		patchHead.InsertSnippet(srcHead)
		applyPatch(srcHead,patchHead)
		srcHead, patchHead := findNextMatch(srcHead, patchHead)
	}
}

Demonstration:
image

@xhd2015
Copy link
Owner Author

xhd2015 commented Jul 22, 2024

With PR #249, a new patching implementation is introduced, just as described above.

In this initial PR, only testing.go has been applied with this new patching method.

Follow on, we will improve the implementation and finally move all patches to this new mechanism.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proposal Proposal new ideas
Projects
Status: Done
Development

No branches or pull requests

1 participant