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

check the store for input before failing (hopefully fix #6383) #7409

Merged
merged 2 commits into from
Dec 9, 2022

Conversation

Radvendii
Copy link
Contributor

This applies the same basic idea as #6572 and #7390, but applied to the problem occurring in #6383.

I am not super familiar with the ++ part of C++, so some things that I'm not sure about:

I have a std::shared_ptr that I'm turning into a regular pointer with .get(). The only thing happening with this pointer is accessing / copying values shortly thereafter. Is this safe? What is the canonical way of doing this?

I had to change auto to const Realisation * because otherwise it was defaulting to a regular Realisation * and then failing later when it needed to be const. I'm not sure if this is the idiomatic way of dealing with this problem.

@Radvendii Radvendii removed the request for review from edolstra December 6, 2022 04:37
@thufschmitt thufschmitt self-assigned this Dec 6, 2022
@Radvendii
Copy link
Contributor Author

Okay this should be cleaner. It took a while because I was running into errors testing this, but it turns out they were unrelated and I just had to do some combination of git clean -f -d -x, make clean and sudo nix-collect-garbage -d. 🤦‍♀️

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/tweag-nix-dev-update-41/23848/1

@Radvendii
Copy link
Contributor Author

@thufschmitt I fixed the problems we discussed about this PR. Care to take another look?

Copy link
Member

@thufschmitt thufschmitt left a comment

Choose a reason for hiding this comment

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

Thanks, looks good 👍

@thufschmitt thufschmitt merged commit 2affb19 into NixOS:master Dec 9, 2022
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.

3 participants