Skip to content

libexpr: allocate ExprPath strings in the allocator #14106

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Sep 30, 2025
Merged

libexpr: allocate ExprPath strings in the allocator #14106

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions src/libexpr/include/nix/expr/nixexpr.hh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,14 +212,16 @@ struct ExprString : Expr
struct ExprPath : Expr
{
ref<SourceAccessor> accessor;
std::string s;
Value v;

ExprPath(ref<SourceAccessor> accessor, std::string s)
ExprPath(std::pmr::polymorphic_allocator<char> & alloc, ref<SourceAccessor> accessor, std::string_view sv)
: accessor(accessor)
, s(std::move(s))
{
v.mkPath(&*accessor, this->s.c_str());
auto len = sv.length();
char * s = alloc.allocate(len + 1);
sv.copy(s, len);
s[len] = '\0';
v.mkPath(&*accessor, s);
}

Value * maybeThunk(EvalState & state, Env & env) override;
Expand Down
2 changes: 1 addition & 1 deletion src/libexpr/nixexpr.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ void ExprString::show(const SymbolTable & symbols, std::ostream & str) const

void ExprPath::show(const SymbolTable & symbols, std::ostream & str) const
{
str << s;
str << v.pathStr();
}

void ExprVar::show(const SymbolTable & symbols, std::ostream & str) const
Expand Down
6 changes: 3 additions & 3 deletions src/libexpr/parser.y
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -392,8 +392,8 @@ path_start
root filesystem accessor, rather than the accessor of the
current Nix expression. */
literal.front() == '/'
? new ExprPath(state->rootFS, std::move(path))
: new ExprPath(state->basePath.accessor, std::move(path));
? new ExprPath(state->alloc, state->rootFS, path)
: new ExprPath(state->alloc, state->basePath.accessor, path);
}
| HPATH {
if (state->settings.pureEval) {
Expand All @@ -403,7 +403,7 @@ path_start
);
}
Path path(getHome() + std::string($1.p + 1, $1.l - 1));
$$ = new ExprPath(ref<SourceAccessor>(state->rootFS), std::move(path));
$$ = new ExprPath(state->alloc, ref<SourceAccessor>(state->rootFS), path);
Copy link
Member

@Mic92 Mic92 Oct 2, 2025

Choose a reason for hiding this comment

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

can we somehow make state->alloc thread local? That would also solve the issue.

Copy link
Contributor

@xokdvium xokdvium Oct 2, 2025

Choose a reason for hiding this comment

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

I doubt having more global variables is a good idea. Maybe as a last resort, but it seems unnecessary at this point if there's other avenues we can explore first?

}
;

Expand Down
Loading