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

[wip] gogensig #790

Closed
wants to merge 67 commits into from
Closed

[wip] gogensig #790

wants to merge 67 commits into from

Conversation

tsingbx
Copy link
Contributor

@tsingbx tsingbx commented Sep 11, 2024

func mangle name: #757

Copy link

codecov bot commented Sep 11, 2024

Codecov Report

Attention: Patch coverage is 98.48837% with 13 lines in your changes missing coverage. Please review.

Project coverage is 97.70%. Comparing base (32f41a0) to head (592e979).

Files with missing lines Patch % Lines
chore/gogensig/convert/package.go 93.22% 4 Missing and 4 partials ⚠️
chore/gogensig/convert/convert.go 91.66% 2 Missing and 1 partial ⚠️
chore/gogensig/convert/builtin.go 97.95% 1 Missing ⚠️
chore/gogensig/convert/type.go 99.28% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #790      +/-   ##
==========================================
+ Coverage   97.57%   97.70%   +0.12%     
==========================================
  Files          20       31      +11     
  Lines        5165     6025     +860     
==========================================
+ Hits         5040     5887     +847     
- Misses        106      114       +8     
- Partials       19       24       +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@tsingbx tsingbx changed the title add unmarshaller [wip] gogensig Sep 11, 2024
@tsingbx tsingbx force-pushed the gogensignew branch 2 times, most recently from d981005 to 6a49f48 Compare September 12, 2024 02:52
Comment on lines 23 to 28
func NewFuncDocComments(funcName string, goFuncName string) *goast.CommentGroup {
txt := "\n//go:linkname " + goFuncName + " " + "C." + funcName
comment := goast.Comment{Text: txt}
commentGroup := goast.CommentGroup{List: []*goast.Comment{&comment}}
return &commentGroup
}
Copy link
Contributor

Choose a reason for hiding this comment

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

这里直接使用llgo:link是更推荐的

Comment on lines 33 to 121
func TestFunc1(t *testing.T) {
bytes, err := readJSONFile("./jsons/func1.json")
if err != nil {
t.Fatal(err)
}
docVisitors := []visitor.DocVisitor{visitor.NewAstConvert("func1")}
p := unmarshal.NewDocFileSetUnmarshaller(docVisitors)
err = p.Unmarshal(bytes)
if err != nil {
t.Fatal(err)
}
}
Copy link
Contributor

@luoliwoshang luoliwoshang Sep 12, 2024

Choose a reason for hiding this comment

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

Mark: 将对这里处理为直接从源代码通过llcppsigfetch的 命令行进行转换,而非读取硬编码的json

goFuncName := toGoFuncName(funcDecl.Name.Name)
p.p.NewFuncDecl(token.NoPos, goFuncName, sig).SetComments(p.p, NewFuncDocComments(funcDecl.Name.Name, goFuncName))
return nil
Copy link
Contributor

Choose a reason for hiding this comment

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

这里Go函数的名字其实应该是接收llcppg.symb.json中的Go Name

{
		"mangle":	"sqlite3_backup_finish",
		"c++":	"sqlite3_backup_finish(sqlite3_backup *)",
		"go":	"backup_finish"
}

Copy link
Contributor

Choose a reason for hiding this comment

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

这个测试文件需要明确标识,并且这个llcppg.cfgllcppg.symb.json 只应该是gogensig测试的一部分,这个文件在llcppg下并不妥

@@ -20,6 +22,15 @@ func NewAstConvert(name string) *AstConvert {
return p
}

func (p *AstConvert) SetupSymbleTableFile(fileName string) error {
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 (p *AstConvert) SetupSymbleTableFile(fileName string) error {
func (p *AstConvert) SetupSymbolTableFile(fileName string) error {

Comment on lines 30 to 34

func llcppsymg(conf []byte) error {
func llcppsymg(conf []byte, out io.Writer) error {
cmd := exec.Command("llcppsymg", "-")
cmd.Stdin = bytes.NewReader(conf)
cmd.Stdout = os.Stdout
cmd.Stdout = out
Copy link
Contributor

Choose a reason for hiding this comment

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

对llcppg的一些修改还是另外起个Pr来做修改

@tsingbx tsingbx force-pushed the gogensignew branch 2 times, most recently from 3c0b88b to c15f178 Compare September 13, 2024 12:51
Comment on lines 11 to 12
docPath := "./_enumtest/spectrum.json"
astConvert := visitor.NewAstConvert("spectrum", docPath)
Copy link
Contributor

Choose a reason for hiding this comment

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

gogensig 对于llcppsigfetch 的输出是直接通过标准输出输入流来做的,也就是说永远不存在一个llcppsigfetch 的输出的文件路径,对于这个converter来说,即使现在需要临时读取一个文件作为输入,也不应该直接将这个NewAstConvert的签名修改为接受docPath

Copy link
Contributor

Choose a reason for hiding this comment

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

conversion 中 指明一下还依赖了这个Pr #757 中的mangledname修改吧,commit中就不用存在这个修改

Copy link
Contributor

Choose a reason for hiding this comment

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

这个就不用上传了,如果是gogensig的一部分,应该在测试用例中

Comment on lines 11 to 16
func TestSpectrum(t *testing.T) {
docPath := "./_testinput/_enumtest/spectrum.h"
astConvert := visitor.NewAstConvert("spectrum", docPath)
Copy link
Contributor

Choose a reason for hiding this comment

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

就目前的设计来说,这里应该接受的是一个llcppg.symb.json的路径,不应该会有头文件了

"github.com/goplus/llgo/chore/gogensig/visitor/symb"
)

func TestLookupSymble(t *testing.T) {
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 TestLookupSymble(t *testing.T) {
func TestLookupSymbol(t *testing.T) {

@tsingbx tsingbx force-pushed the gogensignew branch 9 times, most recently from d7cd226 to f3a8c01 Compare September 25, 2024 01:56
@tsingbx tsingbx closed this Sep 26, 2024
@tsingbx tsingbx reopened this Sep 26, 2024
@tsingbx tsingbx closed this Sep 27, 2024
@tsingbx tsingbx reopened this Sep 27, 2024
}

/*
//TODO
Copy link
Contributor

Choose a reason for hiding this comment

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

这里的todo最好的姿势是类似如下 TODO(xxxx): .......

llgo/cl/import.go

Lines 296 to 300 in 32f41a0

// TODO(xsw): support generic type
func trecvTypeName(t ast.Expr, indices ...ast.Expr) string {
_ = indices
return t.(*ast.Ident).Name
}

@tsingbx tsingbx closed this Sep 30, 2024
@tsingbx tsingbx deleted the gogensignew branch October 25, 2024 03:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants