Skip to content

compiler: go:linkname support method #1152

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 11 commits into from
Sep 30, 2022
Merged

compiler: go:linkname support method #1152

merged 11 commits into from
Sep 30, 2022

Conversation

visualfc
Copy link
Contributor

@visualfc visualfc commented Sep 18, 2022 •
edited
Loading

syntax
go:linkname pkg.type.method
go:linkname pkg.(*type).method

type convert

$linknames["pkg.(*type).method"] = function(s) { return s.method(...[...arguments].slice(1))};

// type convert pkg.(*type).method
	var r = v;
	var ptrType = $ptrType(T);
	if (v.constructor != ptrType) {
		switch (T.kind) {
		case $kindStruct:
			r = $pointerOfStructConversion(v, ptrType);
			break;
		case $kindArray:
			r = new ptrType(v);
			break;
		default:
			r = new ptrType(v.$get,v.$set,v.$target);
		}
	}
// type convert pkg.type.method
	var r = v;
	if (v.constructor != $ptrType(T)) {
		switch (T.kind) {
		case $kindStruct:
			r = $clone(v, T);
			break;
		case $kindSlice:
			r = $convertSliceType(v, T);
			break;
		case $kindComplex64:
		case $kindComplex128:
			r = new T(v.$real, v.$imag);
			break;
		default:
			r = new T(v);
		}
	}

demo

package main

import (
	_ "bytes"
	"fmt"
	_ "unsafe"
)

/*
type bytes.Buffer struct {
	buf      []byte // contents are the bytes buf[off : len(buf)]
	off      int    // read at &buf[off], write at &buf[len(buf)]
	lastRead readOp // last read operation, so that Unread* can work correctly.
}
*/
type buffer struct {
	buf      []byte
	off      int
	lastRead int8
}

//go:linkname _WriteString bytes.(*Buffer).WriteString
func _WriteString(buf *buffer, s string) (n int, err error)

func main() {
	var buf buffer
	_WriteString(&buf, "hello world")
	fmt.Println(buf, string(buf.buf))
}

@nevkontakte
Copy link
Member

Can you give an example when this would be necessary? In general, it seem to me that this would encourage monkey patching, which is often fragile and is a source of difficult to debug issues.

@visualfc
Copy link
Contributor Author

Can you give an example when this would be necessary? In general, it seem to me that this would encourage monkey patching, which is often fragile and is a source of difficult to debug issues.

Yes, it is difficult to implement complete type conversion.

example

https://github.com/goplusjs/reflectx/blob/main/name_js.go#L17
https://github.com/goplusjs/reflectx/blob/main/rtype_js.go#L80

this package provide https://github.com/goplus/igop Interpreter Go/Go+ code for web.
the playground https://github.com/goplusjs/play

but this project build for github.com/goplusjs/gopherjs only run on go1.16.

@nevkontakte
Copy link
Member

Actually, I think I may have misunderstood the intent of this PR.

It allows you to make an unexported method from a third-party package available to your code, right? It does not attempt to replace method implementation in the third-party package with your own? If so, my concern about monkey-patching is not valid.

Another question: does the upstream Go compiler support this style of go:linkname directives?

@visualfc visualfc marked this pull request as draft September 22, 2022 04:29
@visualfc
Copy link
Contributor Author

visualfc commented Sep 22, 2022 •
edited
Loading

Actually, I think I may have misunderstood the intent of this PR.

It allows you to make an unexported method from a third-party package available to your code, right? It does not attempt to replace method implementation in the third-party package with your own? If so, my concern about monkey-patching is not valid.

it can call other package unexport method

Another question: does the upstream Go compiler support this style of go:linkname directives?

go compiler support go:linkname func symbol ( func and mehtod pkg.func/pkg.type.method/pkg.(*type).method )

@visualfc visualfc marked this pull request as ready for review September 22, 2022 13:37
@nevkontakte
Copy link
Member

nevkontakte commented Sep 22, 2022 •
edited
Loading

That makes sense, thanks. I see you are still updating this PR. Please let me know when it is ready for a detailed review, and I'll try to do it without too much delay (hopefully) 😀

Edit: I see you've just marked it ready a few hours ago, never mind then! I'll try to take a close look at it soon.

@visualfc
Copy link
Contributor Author

@nevkontakte add tests and reset commit for this PR.

nevkontakte
nevkontakte requested changes Sep 24, 2022
View reviewed changes
Copy link
Member

@nevkontakte nevkontakte left a comment

Choose a reason for hiding this comment

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

Apologies for the delay, here's the first round of comments. I think overall this looks good, but there are a couple of places that (I think) could be simplified.

nevkontakte
nevkontakte reviewed Sep 29, 2022
View reviewed changes
nevkontakte
nevkontakte reviewed Sep 29, 2022
View reviewed changes
nevkontakte
nevkontakte approved these changes Sep 30, 2022
View reviewed changes
Copy link
Member

@nevkontakte nevkontakte left a comment

Choose a reason for hiding this comment

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

This change looks good to me!

@nevkontakte nevkontakte merged commit 9b96dbe into gopherjs:master Sep 30, 2022
@nevkontakte
Copy link
Member

And thank you again for yet another contribution to GopherJS! ☺

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nevkontakte nevkontakte nevkontakte approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@visualfc @nevkontakte