godef icon indicating copy to clipboard operation
godef copied to clipboard

support mutiple platform/architect dependent definitions

Open tw4452852 opened this issue 10 years ago • 2 comments

when there are multiple platform/architect implements, all of them will be printed.

tw4452852 avatar Jan 30 '15 12:01 tw4452852

I have uploaded a new patch set, please help to review

tw4452852 avatar Feb 10 '15 01:02 tw4452852

I'm sorry for taking so long to get around to reviewing this. I've spent the day today looking at the issue and I'm afraid, though useful, I don't think this change is quite what we need.

A few issues that I noted when working through this:

  1. This change works only on the final symbol and not on intermediate values in the expression. For example, consider the following:
# foo.go
package f
type foo struct {
    X int
}


# foo_windows.go
package f
type foo struct {
    X int
}

# bar.go
package f

var f foo       // def -p on "foo" prints two definitions
var g = f.X     // def -p on "X" prints only one definition

Even though there are actually two alternative definitions of foo.X (which may well be interestingly different from one another), the code will only print one of them because the code for traversing the expression does not know about multiple definitions.

  1. AFAICS the redeclared method will create a circular list when called as:
    p.redeclared(ident, ident.Obj, "as package")

because redeclared assumes that its second argument is the previous declaration, not the current one.

  1. UseAllFiles is not always the right thing to do - in some cases we may actually want to know the exact definition when given a certain set of tags.

  2. token.File.platformDep doesn't really seem like the right place to hold information about platform dependence - to me it seems like somewhere that's too low level for that information. I'm not quite sure where the best place would be though.

  3. I'm not even sure that "platform dependence" is the right thing to be triggering off. It's really "conditional-build" dependence. I see two possible approaches here. We could let the user define a set of possible tag sets and run the definition code for each set. This would be least implementation work, but puts the onus on the user to define appropriate tag sets, which is not ideal - godef is often used for exploring unfamiliar code bases. Another approach, which I think I favour, is to always keep around the complete set of definitions for an identifier - whether they're platform dependent or not. I started along this road trying to make ast.Scope hold a []_Object (I named it type Objects []_Object) but that change is too big for the time I have available now. Another possible approach which I considered was to find the set of tags that might influence the definition location and iterate over all possibilities of them. This could become computationally expensive when there are lots of tags in a given package, but in most cases would probably be trivially cheap.

I'm sorry - this isn't an easy problem, which is why I've put off doing it for so long!

rogpeppe avatar Feb 22 '15 22:02 rogpeppe