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

struct problems by new binary format #352

Closed
zchee opened this issue May 16, 2016 · 17 comments
Closed

struct problems by new binary format #352

zchee opened this issue May 16, 2016 · 17 comments
Labels

Comments

@zchee
Copy link
Contributor

zchee commented May 16, 2016

I found two bugs.

First, It's simple. maybe it's not related new binary format.

package main

type TestStruct struct {
    Name string
}

func NewTestStruct(name string) *TestStruct {
    return &TestStruct{
        Name: name,
    }
}

// This line
func (t *

func (t *TestStruct) Test(name string) error {
    t.Name = name

    return nil
}

If type any character after pointer (e.g. func (t *T), gocode occur PANIC.
But if that situation, works fine.

  1. func (t T
    • no poitner
  2. Remove func (t *TestStruct) Test(name string) error function block
    • e.g. comment out
  3. Move func (t * to end of file
  4. 152 byte offset

3 is this situation.

.
.
func (t *TestStruct) Test(name string) error {
    t.Name = name

    return nil
}

func (t *T

gist: https://gist.github.com/zchee/0789790800f77ad9fa2d06783bc31de2#file-struct-go
byte offset: 151 (include any charactor e.g. T)
cmd: gocode --in ./struct.go autocomplete 151

Next is an edge case(?). It's related new binary format.
I tried #305 (comment) workaround. works fine.

Sorry, I tried several code, but I could not find another case.
If I found other case, will comment.

This sample is simple code for remote connection to delve headless server.

package main

import "github.com/derekparker/delve/service/rpc2"

const addr = "localhost:41222" // d:4 l:12 v:22

func pointerStruct() {
    client := rpc2.NewClient(addr)
    bp, _ := client.GetBreakpoint(-1)

    bp.
}

gist: https://gist.github.com/zchee/0789790800f77ad9fa2d06783bc31de2#file-delve-go
byte offset: 210
cmd: gocode --in ./delve.go autocomplete 210

Does not return any complete word after bp..
Expected behavior is return that struct values.
https://github.com/derekparker/delve/blob/master/service/api/types.go#L35-L69
but actual behavior is Nothing to complete..

Please comment if you need other information.
Thanks.

@nsf
Copy link
Owner

nsf commented May 16, 2016

Ok, I'll take a look at these cases, maybe some time during the week, maybe on the weekend. Thanks for reporting.

@zchee
Copy link
Contributor Author

zchee commented May 16, 2016

@nsf Thanks.

I'm Go beginner, but maybe I'm not busy than your schedule.
I also debug until the weekend, and gather information as much as possible.

@mdempsky
Copy link

FYI, with github.com/mdempsky/gocode, the suggestions I get for func (t * are:

    Found 2 candidates:
      func NewTestStruct(name string) *TestStruct
      type TestStruct struct

(This could perhaps be improved by recognizing that in a function signature context, only types are applicable.)

And for func (t *T are:

    Found 1 candidates:
      type TestStruct struct

For your second using github.com/derekparker/delve/service/rpc2, I get:

    Found 15 candidates:
      var Addr uint64
      var Cond string
      var File string
      var FunctionName string
      var Goroutine bool
      var HitCount map[string]uint64
      var ID int
      var Line int
      var LoadArgs *api.LoadConfig
      var LoadLocals *api.LoadConfig
      var Name string
      var Stacktrace int
      var TotalHitCount uint64
      var Tracepoint bool
      var Variables []string

@zchee
Copy link
Contributor Author

zchee commented May 16, 2016

@mdempsky Thanks for the comment, and I tried it.
Yes, Both cases works fine.
Also on gb project directory structure.

It currently only supports cmd/go builds, because I haven't yet grok'd how gb works.

Is it meaning of gb local packages?
In my case, could not complete a local package. e.g. import "foo/bar" and bar..
but, vendor package was possible. e.g. import "github.com/foo/bar" and bar..

@mdempsky
Copy link

@zchee Cool, glad to hear it works for you.

As for gb support, I essentially mean how to map import paths to .a files. That's unrelated to this issue though, so feel free to follow/comment on mdempsky#1 if you'd like.

@mattn
Copy link
Contributor

mattn commented May 16, 2016

First issue is very simple.

diff --git a/decl.go b/decl.go
index 86c0c29..ad1a42b 100644
--- a/decl.go
+++ b/decl.go
@@ -330,7 +330,10 @@ func method_of(d ast.Decl) string {
                if se, ok := t.X.(*ast.SelectorExpr); ok {
                    return se.Sel.Name
                }
-               return t.X.(*ast.Ident).Name
+               if ident, ok := t.X.(*ast.Ident); ok {
+                   return ident.Name
+               }
+               return ""
            case *ast.Ident:
                return t.Name
            default:

@mattn
Copy link
Contributor

mattn commented May 17, 2016

Second one, I'm not sure, but maybe it's related with importing "C".

@mattn
Copy link
Contributor

mattn commented May 17, 2016

Sorry, It's not related to "C".
If you try below, it will works.

package main

import (
    "github.com/derekparker/delve/service/api"
    "github.com/derekparker/delve/service/rpc2"
)

const addr = "localhost:41222" // d:4 l:12 v:22

func pointerStruct() {
    client := rpc2.NewClient(addr)
    var bp *api.Breakpoint
    bp, _ = client.GetBreakpoint(-1)
    bp.
}

So I guess several imports are not recognized.

@zchee
Copy link
Contributor Author

zchee commented May 18, 2016

@mattn Thanks for workaround!
I have tested first issue on both the go and gb directory structure. work properly.
It a bug?
And, second issue also works if define bp type before assign a function. (both of go and gb)

As you said, it does not seem to be able to get the type of external(not imported) package.

@mattn
Copy link
Contributor

mattn commented May 18, 2016

Yes, It's a bug.

@nsf
Copy link
Owner

nsf commented May 22, 2016

Fixed first panic as suggested by @mattn, thanks for a fix. Will take a look at the second one.

@nsf nsf added the Bug label May 22, 2016
@nsf
Copy link
Owner

nsf commented May 22, 2016

Second case works on gocode which uses textual packages (Go 1.6). If it's indeed fails on Go 1.7 packages, it is a valid regression. I think I know where to look at it, but I will do so when Go 1.7 release is closer.

@zchee
Copy link
Contributor Author

zchee commented May 22, 2016

@nsf Thanks for the quick fix! and @mattn thanks again.
and I understand the second issue. hope it.

I will not close it because you added the bug label in this thread.
Please close it on the timing you think.
Thanks.

@nsf
Copy link
Owner

nsf commented May 22, 2016

Ok, sure. I know where the bug comes from, it just requires a one dedicated day to spend on it, because honestly I have no idea why it happens. But I know where to look. Maybe next weekend I'll spend some time on it, maybe some time later closer to Go 1.7 release. Right now my suggestion: don't use Go 1.7, use Go 1.6 if you can.

@zchee zchee closed this as completed May 22, 2016
@zchee zchee reopened this May 22, 2016
@zchee
Copy link
Contributor Author

zchee commented May 22, 2016

@nsf Sorry, I was closed this :(
reopened.

BTW, As I have mentioned, I'm gopher beginner.
but, gocode's code is a good documentation of Go AST.

I want to do something in return.
You said still do not know the cause, so don't use 1.6, and I will continue to look for the cause points with use the 1.7.
If I found the cause, sends the PR.

fast and dynamically, gocode is a good package reference than godoc.
Thanks for the good tool.

@zchee
Copy link
Contributor Author

zchee commented May 29, 2016

@nsf (@mattn)
Hi,
I tried current latest Go version (go version devel +795809b), the second issue has solved.
rsc has a lot of change the src/cmd/compile/internal/gc, Thereby?

Anyway, now works perfectly.
If it's confirmed, please close this issue.
Thanks.

@zchee
Copy link
Contributor Author

zchee commented Jun 9, 2016

@nsf
This issue has been resolved by #357
Close.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants