From ad5366c2ad43216ac9a61ccb1477ff9859d1a75c Mon Sep 17 00:00:00 2001 From: eldritch horrors Date: Sun, 16 Jun 2024 23:10:09 +0200 Subject: libexpr: pass Exprs as references, not pointers almost all places where Exprs are passed as pointers expect the pointers to be non-null. pass them as references to encode this constraint in the type system as well (and also communicate that Exprs must not be freed). Change-Id: Ia98f166fec3c23151f906e13acb4a0954a5980a2 --- src/libexpr/eval.cc | 60 ++++++++++++++++++++++++++--------------------------- 1 file changed, 30 insertions(+), 30 deletions(-) (limited to 'src/libexpr/eval.cc') diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 4c68fd0b9..c8329a602 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -950,14 +950,14 @@ void EvalState::mkList(Value & v, size_t size) unsigned long nrThunks = 0; -static inline void mkThunk(Value & v, Env & env, Expr * expr) +static inline void mkThunk(Value & v, Env & env, Expr & expr) { v.mkThunk(&env, expr); nrThunks++; } -void EvalState::mkThunk_(Value & v, Expr * expr) +void EvalState::mkThunk_(Value & v, Expr & expr) { mkThunk(v, baseEnv, expr); } @@ -1058,7 +1058,7 @@ void EvalState::mkSingleDerivedPathString( Value * Expr::maybeThunk(EvalState & state, Env & env) { Value * v = state.allocValue(); - mkThunk(*v, env, this); + mkThunk(*v, env, *this); return v; } @@ -1122,7 +1122,7 @@ void EvalState::evalFile(const SourcePath & path_, Value & v, bool mustBeTrivial e = j->second; if (!e) - e = parseExprFromFile(checkSourcePath(resolvedPath)); + e = &parseExprFromFile(checkSourcePath(resolvedPath)); cacheFile(path, resolvedPath, e, v, mustBeTrivial); } @@ -1159,7 +1159,7 @@ void EvalState::cacheFile( if (mustBeTrivial && !(dynamic_cast(e))) error("file '%s' must be an attribute set", path).debugThrow(); - eval(e, v); + eval(*e, v); } catch (Error & e) { addErrorTrace(e, "while evaluating the file '%1%':", resolvedPath.to_string()); throw; @@ -1170,23 +1170,23 @@ void EvalState::cacheFile( } -void EvalState::eval(Expr * e, Value & v) +void EvalState::eval(Expr & e, Value & v) { - e->eval(*this, baseEnv, v); + e.eval(*this, baseEnv, v); } -inline bool EvalState::evalBool(Env & env, Expr * e, const PosIdx pos, std::string_view errorCtx) +inline bool EvalState::evalBool(Env & env, Expr & e, const PosIdx pos, std::string_view errorCtx) { try { Value v; - e->eval(*this, env, v); + e.eval(*this, env, v); if (v.type() != nBool) error( "expected a Boolean but found %1%: %2%", showType(v), ValuePrinter(*this, v, errorPrintOptions) - ).atPos(pos).withFrame(env, *e).debugThrow(); + ).atPos(pos).withFrame(env, e).debugThrow(); return v.boolean; } catch (Error & e) { e.addTrace(positions[pos], errorCtx); @@ -1195,16 +1195,16 @@ inline bool EvalState::evalBool(Env & env, Expr * e, const PosIdx pos, std::stri } -inline void EvalState::evalAttrs(Env & env, Expr * e, Value & v, const PosIdx pos, std::string_view errorCtx) +inline void EvalState::evalAttrs(Env & env, Expr & e, Value & v, const PosIdx pos, std::string_view errorCtx) { try { - e->eval(*this, env, v); + e.eval(*this, env, v); if (v.type() != nAttrs) error( "expected a set but found %1%: %2%", showType(v), ValuePrinter(*this, v, errorPrintOptions) - ).withFrame(env, *e).debugThrow(); + ).withFrame(env, e).debugThrow(); } catch (Error & e) { e.addTrace(positions[pos], errorCtx); throw; @@ -1277,7 +1277,7 @@ void ExprAttrs::eval(EvalState & state, Env & env, Value & v) Value * vAttr; if (hasOverrides && i.second.kind != AttrDef::Kind::Inherited) { vAttr = state.allocValue(); - mkThunk(*vAttr, *i.second.chooseByKind(&env2, &env, inheritEnv), i.second.e); + mkThunk(*vAttr, *i.second.chooseByKind(&env2, &env, inheritEnv), *i.second.e); } else vAttr = i.second.e->maybeThunk(state, *i.second.chooseByKind(&env2, &env, inheritEnv)); env2.values[displ++] = vAttr; @@ -1868,13 +1868,13 @@ void ExprWith::eval(EvalState & state, Env & env, Value & v) void ExprIf::eval(EvalState & state, Env & env, Value & v) { // We cheat in the parser, and pass the position of the condition as the position of the if itself. - (state.evalBool(env, cond, pos, "while evaluating a branch condition") ? then : else_)->eval(state, env, v); + (state.evalBool(env, *cond, pos, "while evaluating a branch condition") ? *then : *else_).eval(state, env, v); } void ExprAssert::eval(EvalState & state, Env & env, Value & v) { - if (!state.evalBool(env, cond, pos, "in the condition of the assert statement")) { + if (!state.evalBool(env, *cond, pos, "in the condition of the assert statement")) { std::ostringstream out; cond->show(state.symbols, out); state.error("assertion '%1%' failed", out.str()).atPos(pos).withFrame(env, *this).debugThrow(); @@ -1885,7 +1885,7 @@ void ExprAssert::eval(EvalState & state, Env & env, Value & v) void ExprOpNot::eval(EvalState & state, Env & env, Value & v) { - v.mkBool(!state.evalBool(env, e, getPos(), "in the argument of the not operator")); // XXX: FIXME: ! + v.mkBool(!state.evalBool(env, *e, getPos(), "in the argument of the not operator")); // XXX: FIXME: ! } @@ -1907,27 +1907,27 @@ void ExprOpNEq::eval(EvalState & state, Env & env, Value & v) void ExprOpAnd::eval(EvalState & state, Env & env, Value & v) { - v.mkBool(state.evalBool(env, e1, pos, "in the left operand of the AND (&&) operator") && state.evalBool(env, e2, pos, "in the right operand of the AND (&&) operator")); + v.mkBool(state.evalBool(env, *e1, pos, "in the left operand of the AND (&&) operator") && state.evalBool(env, *e2, pos, "in the right operand of the AND (&&) operator")); } void ExprOpOr::eval(EvalState & state, Env & env, Value & v) { - v.mkBool(state.evalBool(env, e1, pos, "in the left operand of the OR (||) operator") || state.evalBool(env, e2, pos, "in the right operand of the OR (||) operator")); + v.mkBool(state.evalBool(env, *e1, pos, "in the left operand of the OR (||) operator") || state.evalBool(env, *e2, pos, "in the right operand of the OR (||) operator")); } void ExprOpImpl::eval(EvalState & state, Env & env, Value & v) { - v.mkBool(!state.evalBool(env, e1, pos, "in the left operand of the IMPL (->) operator") || state.evalBool(env, e2, pos, "in the right operand of the IMPL (->) operator")); + v.mkBool(!state.evalBool(env, *e1, pos, "in the left operand of the IMPL (->) operator") || state.evalBool(env, *e2, pos, "in the right operand of the IMPL (->) operator")); } void ExprOpUpdate::eval(EvalState & state, Env & env, Value & v) { Value v1, v2; - state.evalAttrs(env, e1, v1, pos, "in the left operand of the update (//) operator"); - state.evalAttrs(env, e2, v2, pos, "in the right operand of the update (//) operator"); + state.evalAttrs(env, *e1, v1, pos, "in the left operand of the update (//) operator"); + state.evalAttrs(env, *e2, v2, pos, "in the right operand of the update (//) operator"); state.nrOpUpdates++; @@ -2748,39 +2748,39 @@ SourcePath resolveExprPath(SourcePath path) } -Expr * EvalState::parseExprFromFile(const SourcePath & path) +Expr & EvalState::parseExprFromFile(const SourcePath & path) { return parseExprFromFile(path, staticBaseEnv); } -Expr * EvalState::parseExprFromFile(const SourcePath & path, std::shared_ptr & staticEnv) +Expr & EvalState::parseExprFromFile(const SourcePath & path, std::shared_ptr & staticEnv) { auto buffer = path.readFile(); // readFile hopefully have left some extra space for terminators buffer.append("\0\0", 2); - return parse(buffer.data(), buffer.size(), Pos::Origin(path), path.parent(), staticEnv); + return *parse(buffer.data(), buffer.size(), Pos::Origin(path), path.parent(), staticEnv); } -Expr * EvalState::parseExprFromString(std::string s_, const SourcePath & basePath, std::shared_ptr & staticEnv) +Expr & EvalState::parseExprFromString(std::string s_, const SourcePath & basePath, std::shared_ptr & staticEnv) { // NOTE this method (and parseStdin) must take care to *fully copy* their input // into their respective Pos::Origin until the parser stops overwriting its input // data. auto s = make_ref(s_); s_.append("\0\0", 2); - return parse(s_.data(), s_.size(), Pos::String{.source = s}, basePath, staticEnv); + return *parse(s_.data(), s_.size(), Pos::String{.source = s}, basePath, staticEnv); } -Expr * EvalState::parseExprFromString(std::string s, const SourcePath & basePath) +Expr & EvalState::parseExprFromString(std::string s, const SourcePath & basePath) { return parseExprFromString(std::move(s), basePath, staticBaseEnv); } -Expr * EvalState::parseStdin() +Expr & EvalState::parseStdin() { // NOTE this method (and parseExprFromString) must take care to *fully copy* their // input into their respective Pos::Origin until the parser stops overwriting its @@ -2790,7 +2790,7 @@ Expr * EvalState::parseStdin() // drainFD should have left some extra space for terminators auto s = make_ref(buffer); buffer.append("\0\0", 2); - return parse(buffer.data(), buffer.size(), Pos::Stdin{.source = s}, rootPath(CanonPath::fromCwd()), staticBaseEnv); + return *parse(buffer.data(), buffer.size(), Pos::Stdin{.source = s}, rootPath(CanonPath::fromCwd()), staticBaseEnv); } -- cgit v1.2.3