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

SymbolStr: Remove std::string conversion #11082

Merged
merged 1 commit into from
Jul 12, 2024

Conversation

edolstra
Copy link
Member

Motivation

This allows the symbol table to be stored as something other than std::strings. Cherry-picked from #10938 where the symbol table is stored as contiguous null-terminated strings.

Context

Priorities and Process

Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

This refactoring allows the symbol table to be stored as something
other than std::strings.
@github-actions github-actions bot added new-cli Relating to the "nix" command c api Nix as a C library with a stable interface labels Jul 11, 2024
Copy link
Contributor

@fricklerhandwerk fricklerhandwerk left a comment

Choose a reason for hiding this comment

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

Nice

@roberth
Copy link
Member

roberth commented Jul 11, 2024

Looks alright. As a matter of practicality, do we merge this or #11072 first? Probably conflicts.

@edolstra edolstra merged commit b57c361 into NixOS:master Jul 12, 2024
11 checks passed
@edolstra edolstra deleted the symbol-table-string-view branch July 12, 2024 11:39
@edolstra
Copy link
Member Author

#11072 is a big one that might need more review? It doesn't seem to conflict though.

@roberth
Copy link
Member

roberth commented Jul 12, 2024

Tom has already reviewed most of it. Only the lexer part needs an extra pair of eyes.

I kinda expected a conflict in the text, but I'll rebase it to make sure CI has run on the "merge".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c api Nix as a C library with a stable interface new-cli Relating to the "nix" command
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants