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

pkg/proc: implement composite literals #3691

Closed

Conversation

firelizzard18
Copy link

This change implements composite literals for struct, slice, array, and map types.

Fixes #1465

This change adds support for evaluating composite
struct literals.

Updates go-delve#1465
This change adds support for slice and array literals.

Updates go-delve#1465
This change adds support for map literals.

Fixes go-delve#1465
@firelizzard18
Copy link
Author

My use case is the original ask in #1465 - accessing map elements (e.g. a watch expression in vscode). I'm not sure how much value there is in map and slice literals (since those can't me map keys). The map literals my code makes are at least somewhat broken - they can't be iterated since Variable.Addr is zero. I'm not sure how much else is broken.

Copy link
Member

@aarzilli aarzilli left a comment

Choose a reason for hiding this comment

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

This is the wrong approach. It works if all you want to do with the literal is either nothing or a map lookup but taking its address, using it in assignments or passing them to function calls will be broken, either in obvious or subtle ways (for example if the return value of a function call is assigned to the field of a struct we need to make sure that any pointers it might contain are visible to the garbage collector of the target program).

With the way call injection currently works this is approximately impossible, which is why that issue hasn't been closed in such a long time. I've been working on making this possible however.

@firelizzard18
Copy link
Author

I assume it's the Variable construction in (*EvalScope).evalCompositeLit that's broken? Should I close this PR?

@aarzilli
Copy link
Member

aarzilli commented Apr 2, 2024

Basically, you are creating a Variable that can only be used in very specific expressions. As I said, there isn't a really way to make this work properly, at the moment. Sorry.

@firelizzard18
Copy link
Author

As I said, there isn't a really way to make this work properly, at the moment.

Sure, I get that and I'm not surprised my 'solution' doesn't work. I'm not clear on what you'd prefer to do with this PR: close it or leave it open/as a draft until it can be done properly. It doesn't make much of a difference to me either way.

@aarzilli
Copy link
Member

aarzilli commented Apr 2, 2024

You should close it, I already have a version privately that does this with the allocation and it's very different from this.

@AlexanderMakarov
Copy link

You should close it, I already have a version privately that does this with the allocation and it's very different from this.

@aarzilli are you planning to release your version? Is it possible to download it somewhere?

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.

Support map evaluation by composite literal keys
3 participants