refactor: Add Value types, use std::span for list iteration
#9370
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
**`Value` and `const`** These two deserve some explanation. We'll get to lists later. Values can normally be thought of as immutable, except they are are also the vehicle for call by need, which must be implemented using mutation. This circumstance makes a `const Value` a rather useless thing: - If it's a thunk, you can't evaluate it, except by copying, but that would not be call by need. - If it's not a thunk, you know the type, so the method that acquired it for you should have returned something more specific, such as a `const Bindings &` (which actually does make sense because that's an immutable span of pointers to mutable `Value`s. - If you don't care about the type yet, you might establish the convention that `const Value` means `deepSeq`-ed data, but this is hardly useful and not actually as safe as you would supposedly want to trust it to be - just convention. **Lists** `std::span` is a tuple of pointer and size - just what we need. We don't return them as `const Value`, because considering the first bullet point we discussed before, we'd have to force all the list values, which isn't what we want. So what we end up with is a nice representation of a list in weak head normal form: the spine is immutable, but the items may need some evaluation later.
roberth
added
the
language
Ericson2314
approved these changes
Nov 17, 2023
|
(But @tomberek is reviewing it too :)) |
|
A while ago we had to revert a use of |
I don't think we need to support that anymore. We have GCC 12.2 for aarch64-linux, in the flake here and in NixOS 23.05. |
|
Yeah the 23.05 bump solved that. |
tomberek
approved these changes
Nov 20, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
I've done some work on exception safety in the evaluator that's resulting in some refactoring. I'm not done with it yet, but I believe these commits should be uncontroversial and I figured
StringWithContextmight be semi-useful for @tomberek's work on string performance.Changes:
Valueunion. They aren't used yet, but can be used to write more type safe helper functions.std::spanfor accessing list values. That's a view of a contiguous memory region a dynamic number of instances of a certain type.string_viewis like aspan<char>.constis used. I've explained it in the second to last commit message.nix-envis also basically a no-op, but makes it so that all list initialization happens through EvalState, which will be useful later.Context
Reasons why I'm looking into exception safety:
Priorities
Add 👍 to pull requests you find important.