proc: make PushPackageVarOrSelect check local variables first
Fixes #4179
To be honest I'm not sure it's worth doing this. It matches better the semantics of Go but It brings in some complication and it's mostly about collisions between local variables and global variables (which are rare anyway).
This PR seems to address another issue I discovered the other day. When the local structure variable and the package name in dwarf are the same, when calling the method of the local structure variable, it will be displayed that the method cannot be found. 🤣 I'll test it tonight when I have some free time to see if I can solve another problem.
I tried debugging the unit tests using the Delve command fixed by this patch and found other issues.
When I try to call ctx.Value("ok"), I receive an error message: could not find symbol. This seems to be because the local variable name conflicts with another package of the same name.
I took a cursory look at the dlv source code and its associated fncall logs. When compiling to an ast expression, ctx is replaced by another name.
example:
fncall logs:
What's code.byted.org/gin/ginex/ctx.Value?
the code.byted.org/gin/ginex/ctx is the package name. No code.byted.org/gin/ginex/ctx.Value.
I've determined that this code is causing the problem. But I'm not sure if this scenario makes sense.
https://github.com/go-delve/delve/blob/b271ba362b43fa0cdaff0899cf076393a436c0d9/pkg/proc/bininfo.go#L3002-L3009
Should be fixed.
I used your patch and it worked fine locally.
To be honest I'm not sure it's worth doing this. It matches better the semantics of Go but It brings in some complication and it's mostly about collisions between local variables and global variables (which are rare anyway).
I've been holding off on review due to this concern. Do you still feel this way?
I've been holding off on review due to this concern. Do you still feel this way?
I'm inclined to merge this.