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

Fix assignment to optional payload #3222

Closed
wants to merge 1 commit into from
Closed

Conversation

LemonBoy
Copy link
Contributor

My first foray into the wild result-location code was not that pleasant, for the love of whatever deity you believe in add some comments (and/or document how the mechanism works).

I have no damn clue about what written refers to but resetting it lets EndExpr do its job and write out the value into the result slot.

It's probably needed also in the if branch below since a similar unwrapping operation is performed.

Closes #3081

@andrewrk andrewrk added this to the 0.5.0 milestone Sep 19, 2019
@andrewrk
Copy link
Member

andrewrk commented Sep 24, 2019

My first foray into the wild result-location code was not that pleasant, for the love of whatever deity you believe in add some comments (and/or document how the mechanism works).

to be honest I'm just as scared of that code as you are.

I think how it works is horrible, and doesn't make sense at all.

my only hope right now is to have some kind of shower epiphany when doing the self-hosted implementation, and then backport that to stage1.

this is the third attempt at result location semantics that I've done. the first 2 failed. I'm at the edge of my capabilities and I cannot do any better than this and I need help

dramatics aside, I would be happy to explain more about how it works, to the best of my understanding. but it's not pretty.

also it would only get more complicated with #2761 and #2765

@andrewrk
Copy link
Member

Landed in 93367ad

@andrewrk andrewrk closed this Sep 24, 2019
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.

Problem with array of optional union values
2 participants