lix-project/lix
20
57
Fork 31
Code Issues 347 Code Review (Gerrit) Projects 3 Releases Wiki Activity

libexpr: pass Exprs as references, not pointers

Browse source 
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
This commit is contained in:
eldritch horrors 2024-06-16 23:10:09 +02:00
parent b8f49a8eaf
commit ad5366c2ad
14 changed files with 85 additions and 75 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
src/libcmd/installables.cc
View file

@ -214,7 +214,7 @@ void SourceExprCommand::completeInstallable(AddCompletions & completions, std::s
evalSettings.pureEval = false;
auto state = getEvalState();
Expr *e = state->parseExprFromFile(
Expr & e = state->parseExprFromFile(
resolveExprPath(state->checkSourcePath(lookupFileArg(*state, *file)))
);
@ -434,13 +434,13 @@ Installables SourceExprCommand::parseInstallables(
auto vFile = state->allocValue();
if (file == "-") {
auto e = state->parseStdin();
auto & e = state->parseStdin();
state->eval(e, *vFile);
}
else if (file)
state->evalFile(lookupFileArg(*state, *file), *vFile);
else {
auto e = state->parseExprFromString(*expr, state->rootPath(CanonPath::fromCwd()));
auto & e = state->parseExprFromString(*expr, state->rootPath(CanonPath::fromCwd()));
state->eval(e, *vFile);
}

20
src/libcmd/repl.cc
View file

@ -155,7 +155,7 @@ struct NixRepl
void reloadFiles();
void addAttrsToScope(Value & attrs);
void addVarToScope(const Symbol name, Value & v);
Expr * parseString(std::string s);
Expr & parseString(std::string s);
void evalString(std::string s, Value & v);
void loadDebugTraceEnv(DebugTrace & dt);
@ -428,9 +428,9 @@ StringSet NixRepl::completePrefix(const std::string & prefix)
auto expr = cur.substr(0, dot);
auto cur2 = cur.substr(dot + 1);
Expr * e = parseString(expr);
Expr & e = parseString(expr);
Value v;
e->eval(*state, *env, v);
e.eval(*state, *env, v);
state->forceAttrs(v, noPos, "while evaluating an attrset for the purpose of completion (this error should not be displayed; file an issue?)");
for (auto & i : *v.attrs) {
@ -821,7 +821,7 @@ ProcessLineResult NixRepl::processLine(std::string line)
line[p + 1] != '=' &&
isVarName(name = removeWhitespace(line.substr(0, p))))
{
Expr * e = parseString(line.substr(p + 1));
Expr & e = parseString(line.substr(p + 1));
Value & v(*state->allocValue());
v.mkThunk(env, e);
addVarToScope(state->symbols.create(name), v);
@ -946,7 +946,7 @@ Value * NixRepl::getReplOverlaysEvalFunction()
auto code =
#include "repl-overlays.nix.gen.hh"
;
auto expr = state->parseExprFromString(
auto & expr = state->parseExprFromString(
code,
SourcePath(evalReplInitFilesPath),
state->staticBaseEnv
@ -1054,7 +1054,7 @@ Value * NixRepl::bindingsToAttrs()
}
Expr * NixRepl::parseString(std::string s)
Expr & NixRepl::parseString(std::string s)
{
return state->parseExprFromString(std::move(s), state->rootPath(CanonPath::fromCwd()), staticEnv);
}
@ -1062,16 +1062,16 @@ Expr * NixRepl::parseString(std::string s)
void NixRepl::evalString(std::string s, Value & v)
{
Expr * e = parseString(s);
e->eval(*state, *env, v);
Expr & e = parseString(s);
e.eval(*state, *env, v);
state->forceValue(v, v.determinePos(noPos));
}
Value * NixRepl::evalFile(SourcePath & path)
{
auto expr = state->parseExprFromFile(path, staticEnv);
auto & expr = state->parseExprFromFile(path, staticEnv);
Value * result(state->allocValue());
expr->eval(*state, *env, *result);
expr.eval(*state, *env, *result);
state->forceValue(*result, result->determinePos(noPos));
return result;
}

4
src/libexpr/eval-inline.hh
View file

@ -86,11 +86,11 @@ void EvalState::forceValue(Value & v, const PosIdx pos)
{
if (v.isThunk()) {
Env * env = v.thunk.env;
Expr * expr = v.thunk.expr;
Expr & expr = *v.thunk.expr;
try {
v.mkBlackhole();
//checkInterrupt();
expr->eval(*this, *env, v);
expr.eval(*this, *env, v);
} catch (...) {
v.mkThunk(env, expr);
tryFixupBlackHolePos(v, pos);

60
src/libexpr/eval.cc
View file

@ -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<ExprAttrs *>(e)))
error<EvalError>("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<TypeError>(
"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<TypeError>(
"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<AssertionError>("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> & staticEnv)
Expr & EvalState::parseExprFromFile(const SourcePath & path, std::shared_ptr<StaticEnv> & 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> & staticEnv)
Expr & EvalState::parseExprFromString(std::string s_, const SourcePath & basePath, std::shared_ptr<StaticEnv> & 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<std::string>(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<std::string>(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);
}

20
src/libexpr/eval.hh
View file

@ -340,16 +340,16 @@ public:
/**
* Parse a Nix expression from the specified file.
*/
Expr * parseExprFromFile(const SourcePath & path);
Expr * parseExprFromFile(const SourcePath & path, std::shared_ptr<StaticEnv> & staticEnv);
Expr & parseExprFromFile(const SourcePath & path);
Expr & parseExprFromFile(const SourcePath & path, std::shared_ptr<StaticEnv> & staticEnv);
/**
* Parse a Nix expression from the specified string.
*/
Expr * parseExprFromString(std::string s, const SourcePath & basePath, std::shared_ptr<StaticEnv> & staticEnv);
Expr * parseExprFromString(std::string s, const SourcePath & basePath);
Expr & parseExprFromString(std::string s, const SourcePath & basePath, std::shared_ptr<StaticEnv> & staticEnv);
Expr & parseExprFromString(std::string s, const SourcePath & basePath);
Expr * parseStdin();
Expr & parseStdin();
/**
* Evaluate an expression read from the given file to normal
@ -390,15 +390,15 @@ public:
*
* @param [out] v The resulting is stored here.
*/
void eval(Expr * e, Value & v);
void eval(Expr & e, Value & v);
/**
* Evaluation the expression, then verify that it has the expected
* type.
*/
inline bool evalBool(Env & env, Expr * e);
inline bool evalBool(Env & env, Expr * e, const PosIdx pos, std::string_view errorCtx);
inline void evalAttrs(Env & env, Expr * e, Value & v, const PosIdx pos, std::string_view errorCtx);
inline bool evalBool(Env & env, Expr & e);
inline bool evalBool(Env & env, Expr & e, const PosIdx pos, std::string_view errorCtx);
inline void evalAttrs(Env & env, Expr & e, Value & v, const PosIdx pos, std::string_view errorCtx);
/**
* If `v` is a thunk, enter it and overwrite `v` with the result
@ -619,7 +619,7 @@ public:
}
void mkList(Value & v, size_t length);
void mkThunk_(Value & v, Expr * expr);
void mkThunk_(Value & v, Expr & expr);
void mkPos(Value & v, PosIdx pos);
/**

9
src/libexpr/nixexpr.hh
View file

@ -42,12 +42,21 @@ std::string showAttrPath(const SymbolTable & symbols, const AttrPath & attrPath)
struct Expr
{
protected:
Expr(Expr &&) = default;
Expr & operator=(Expr &&) = default;
public:
struct AstSymbols {
Symbol sub, lessThan, mul, div, or_, findFile, nixPath, body;
};
Expr() = default;
Expr(const Expr &) = delete;
Expr & operator=(const Expr &) = delete;
virtual ~Expr() { };
virtual void show(const SymbolTable & symbols, std::ostream & str) const;
virtual void bindVars(EvalState & es, const std::shared_ptr<const StaticEnv> & env);
virtual void eval(EvalState & state, Env & env, Value & v);

10
src/libexpr/primops.cc
View file

@ -244,9 +244,9 @@ static void import(EvalState & state, const PosIdx pos, Value & vPath, Value * v
// args[0]->attrs is already sorted.
debug("evaluating file '%1%'", path);
Expr * e = state.parseExprFromFile(resolveExprPath(path), staticEnv);
Expr & e = state.parseExprFromFile(resolveExprPath(path), staticEnv);
e->eval(state, *env, v);
e.eval(state, *env, v);
}
}
}
@ -390,13 +390,13 @@ void prim_exec(EvalState & state, const PosIdx pos, Value * * args, Value & v)
auto output = runProgram(program, true, commandArgs);
Expr * parsed;
try {
parsed = state.parseExprFromString(std::move(output), state.rootPath(CanonPath::root));
parsed = &state.parseExprFromString(std::move(output), state.rootPath(CanonPath::root));
} catch (Error & e) {
e.addTrace(state.positions[pos], "while parsing the output from '%1%'", program);
throw;
}
try {
state.eval(parsed, v);
state.eval(*parsed, v);
} catch (Error & e) {
e.addTrace(state.positions[pos], "while evaluating the output from '%1%'", program);
throw;
@ -4494,7 +4494,7 @@ void EvalState::createBaseEnv()
// the parser needs two NUL bytes as terminators; one of them
// is implied by being a C string.
"\0";
eval(parse(code, sizeof(code), derivationInternal, {CanonPath::root}, staticBaseEnv), *vDerivation);
eval(*parse(code, sizeof(code), derivationInternal, {CanonPath::root}, staticBaseEnv), *vDerivation);
}

4
src/libexpr/value.hh
View file

@ -323,11 +323,11 @@ public:
}
}
inline void mkThunk(Env * e, Expr * ex)
inline void mkThunk(Env * e, Expr & ex)
{
internalType = tThunk;
thunk.env = e;
thunk.expr = ex;
thunk.expr = &ex;
}
inline void mkApp(Value * l, Value * r)

4
src/nix-build/nix-build.cc
View file

@ -234,7 +234,7 @@ static void main_nix_build(int argc, char * * argv)
DrvInfos drvs;
/* Parse the expressions. */
std::vector<Expr *> exprs;
std::vector<std::reference_wrapper<Expr>> exprs;
if (readStdin)
exprs = {state->parseStdin()};
@ -337,7 +337,7 @@ static void main_nix_build(int argc, char * * argv)
if (!shell) {
try {
auto expr = state->parseExprFromString(
auto & expr = state->parseExprFromString(
"(import <nixpkgs> {}).bashInteractive",
state->rootPath(CanonPath::fromCwd()));

2
src/nix-env/nix-env.cc
View file

@ -413,7 +413,7 @@ static void queryInstSources(EvalState & state,
loadSourceExpr(state, *instSource.nixExprPath, vArg);
for (auto & i : args) {
Expr * eFun = state.parseExprFromString(i, state.rootPath(CanonPath::fromCwd()));
Expr & eFun = state.parseExprFromString(i, state.rootPath(CanonPath::fromCwd()));
Value vFun, vTmp;
state.eval(eFun, vFun);
vTmp.mkApp(&vFun, &vArg);

8
src/nix-instantiate/nix-instantiate.cc
View file

@ -27,10 +27,10 @@ enum OutputKind { okPlain, okXML, okJSON };
void processExpr(EvalState & state, const Strings & attrPaths,
bool parseOnly, bool strict, Bindings & autoArgs,
bool evalOnly, OutputKind output, bool location, Expr * e)
bool evalOnly, OutputKind output, bool location, Expr & e)
{
if (parseOnly) {
e->show(state.symbols, std::cout);
e.show(state.symbols, std::cout);
std::cout << "\n";
return;
}
@ -175,14 +175,14 @@ static int main_nix_instantiate(int argc, char * * argv)
}
if (readStdin) {
Expr * e = state->parseStdin();
Expr & e = state->parseStdin();
processExpr(*state, attrPaths, parseOnly, strict, autoArgs,
evalOnly, outputKind, xmlOutputSourceLocation, e);
} else if (files.empty() && !fromArgs)
files.push_back("./default.nix");
for (auto & i : files) {
Expr * e = fromArgs
Expr & e = fromArgs
? state->parseExprFromString(i, state->rootPath(CanonPath::fromCwd()))
: state->parseExprFromFile(resolveExprPath(state->checkSourcePath(lookupFileArg(*state, i))));
processExpr(*state, attrPaths, parseOnly, strict, autoArgs,

2
src/nix/main.cc
View file

@ -234,7 +234,7 @@ static void showHelp(std::vector<std::string> subcommand, NixArgs & toplevel)
auto vUtils = state.allocValue();
state.cacheFile(
CanonPath("/utils.nix"), CanonPath("/utils.nix"),
state.parseExprFromString(
&state.parseExprFromString(
#include "utils.nix.gen.hh"
, CanonPath::root),
*vUtils);

3
tests/unit/libexpr-support/tests/libexpr.hh
View file

@ -28,8 +28,7 @@ namespace nix {
}
Value eval(std::string input, bool forceValue = true) {
Value v;
Expr * e = state.parseExprFromString(input, state.rootPath(CanonPath::root));
assert(e);
Expr & e = state.parseExprFromString(input, state.rootPath(CanonPath::root));
state.eval(e, v);
if (forceValue)
state.forceValue(v, noPos);

8
tests/unit/libexpr/value/print.cc
View file

@ -91,7 +91,8 @@ TEST_F(ValuePrintingTests, tList)
TEST_F(ValuePrintingTests, vThunk)
{
Value vThunk;
vThunk.mkThunk(nullptr, nullptr);
ExprInt e(0);
vThunk.mkThunk(nullptr, e);
test(vThunk, "«thunk»");
}
@ -521,7 +522,7 @@ TEST_F(ValuePrintingTests, ansiColorsAssert)
ExprAssert expr(noPos, &eFalse, &eInt);
Value v;
state.mkThunk_(v, &expr);
state.mkThunk_(v, expr);
test(v,
ANSI_RED "«error: assertion 'false' failed»" ANSI_NORMAL,
@ -621,7 +622,8 @@ TEST_F(ValuePrintingTests, ansiColorsPrimOpApp)
TEST_F(ValuePrintingTests, ansiColorsThunk)
{
Value v;
v.mkThunk(nullptr, nullptr);
ExprInt e(0);
v.mkThunk(nullptr, e);
test(v,
ANSI_MAGENTA "«thunk»" ANSI_NORMAL,