diff --git a/third_party/nix/src/libexpr/eval.cc b/third_party/nix/src/libexpr/eval.cc index 6d77928d7..6df7e66b6 100644 --- a/third_party/nix/src/libexpr/eval.cc +++ b/third_party/nix/src/libexpr/eval.cc @@ -6,6 +6,7 @@ #include #include #include +#include #include #include @@ -24,6 +25,7 @@ #include "libutil/hash.hh" #include "libutil/json.hh" #include "libutil/util.hh" +#include "libutil/visitor.hh" namespace nix { @@ -185,13 +187,15 @@ static void* oomHandler(size_t requested) { #endif static Symbol getName(const AttrName& name, EvalState& state, Env& env) { - if (name.symbol.set()) { - return name.symbol; - } - Value nameValue; - name.expr->eval(state, env, nameValue); - state.forceStringNoCtx(nameValue); - return state.symbols.Create(nameValue.string.s); + return std::visit( + util::overloaded{[&](const Symbol& name) -> Symbol { return name; }, + [&](Expr* expr) -> Symbol { + Value nameValue; + expr->eval(state, env, nameValue); + state.forceStringNoCtx(nameValue); + return state.symbols.Create(nameValue.string.s); + }}, + name); } static bool gcInitialised = false; @@ -890,12 +894,7 @@ static std::string showAttrPath(EvalState& state, Env& env, } else { first = false; } - try { - out << getName(i, state, env); - } catch (Error& e) { - assert(!i.symbol.set()); - out << "\"${" << *i.expr << "}\""; - } + out << getName(i, state, env); } return out.str(); } diff --git a/third_party/nix/src/libexpr/nixexpr.cc b/third_party/nix/src/libexpr/nixexpr.cc index 28d8eee7a..2de37f50e 100644 --- a/third_party/nix/src/libexpr/nixexpr.cc +++ b/third_party/nix/src/libexpr/nixexpr.cc @@ -1,9 +1,11 @@ #include "libexpr/nixexpr.hh" #include +#include #include "libstore/derivations.hh" #include "libutil/util.hh" +#include "libutil/visitor.hh" namespace nix { @@ -198,17 +200,17 @@ std::ostream& operator<<(std::ostream& str, const Pos& pos) { std::string showAttrPath(const AttrPath& attrPath) { std::ostringstream out; bool first = true; - for (auto& i : attrPath) { + for (auto& attr : attrPath) { if (!first) { out << '.'; } else { first = false; } - if (i.symbol.set()) { - out << i.symbol; - } else { - out << "\"${" << *i.expr << "}\""; - } + + std::visit(util::overloaded{ + [&](const Symbol& sym) { out << sym; }, + [&](const Expr* expr) { out << "\"${" << *expr << "}\""; }}, + attr); } return out.str(); } @@ -268,8 +270,8 @@ void ExprSelect::bindVars(const StaticEnv& env) { def->bindVars(env); } for (auto& i : attrPath) { - if (!i.symbol.set()) { - i.expr->bindVars(env); + if (auto* expr = std::get_if(&i)) { + (*expr)->bindVars(env); } } } @@ -277,8 +279,8 @@ void ExprSelect::bindVars(const StaticEnv& env) { void ExprOpHasAttr::bindVars(const StaticEnv& env) { e->bindVars(env); for (auto& i : attrPath) { - if (!i.symbol.set()) { - i.expr->bindVars(env); + if (auto* expr = std::get_if(&i)) { + (*expr)->bindVars(env); } } } diff --git a/third_party/nix/src/libexpr/nixexpr.hh b/third_party/nix/src/libexpr/nixexpr.hh index 0bf524518..7ec6f7f17 100644 --- a/third_party/nix/src/libexpr/nixexpr.hh +++ b/third_party/nix/src/libexpr/nixexpr.hh @@ -1,6 +1,7 @@ #pragma once #include +#include #include "libexpr/symbol-table.hh" #include "libexpr/value.hh" @@ -60,12 +61,7 @@ class EvalState; struct StaticEnv; /* An attribute path is a sequence of attribute names. */ -struct AttrName { - Symbol symbol; - Expr* expr; - AttrName(const Symbol& s) : symbol(s){}; - AttrName(Expr* e) : expr(e){}; -}; +using AttrName = std::variant; typedef std::vector AttrPath; diff --git a/third_party/nix/src/libexpr/parser.y b/third_party/nix/src/libexpr/parser.y index fe8759f3c..eb3f92db7 100644 --- a/third_party/nix/src/libexpr/parser.y +++ b/third_party/nix/src/libexpr/parser.y @@ -16,6 +16,7 @@ #ifndef BISON_HEADER #define BISON_HEADER +#include #include "libutil/util.hh" #include "libexpr/nixexpr.hh" #include "libexpr/eval.hh" @@ -84,30 +85,39 @@ static void addAttr(ExprAttrs * attrs, AttrPath & attrPath, // Checking attrPath validity. // =========================== for (i = attrPath.begin(); i + 1 < attrPath.end(); i++) { - if (i->symbol.set()) { - ExprAttrs::AttrDefs::iterator j = attrs->attrs.find(i->symbol); + if (const auto* sym = std::get_if(&(*i)); sym && sym->set()) { + ExprAttrs::AttrDefs::iterator j = attrs->attrs.find(*sym); if (j != attrs->attrs.end()) { if (!j->second.inherited) { ExprAttrs * attrs2 = dynamic_cast(j->second.e); if (!attrs2) { dupAttr(attrPath, pos, j->second.pos); } attrs = attrs2; - } else + } else { dupAttr(attrPath, pos, j->second.pos); + } } else { - ExprAttrs * nested = new ExprAttrs; - attrs->attrs[i->symbol] = ExprAttrs::AttrDef(nested, pos); + ExprAttrs* nested = new ExprAttrs; + attrs->attrs[*sym] = ExprAttrs::AttrDef(nested, pos); attrs = nested; } } else { - ExprAttrs *nested = new ExprAttrs; - attrs->dynamicAttrs.push_back(ExprAttrs::DynamicAttrDef(i->expr, nested, pos)); - attrs = nested; + // Yes, this code does not handle all conditions + // exhaustively. We use std::get to throw if the condition + // that isn't covered happens, which is potentially a + // behaviour change from the previous default constructed + // Symbol. It should alert us about anything untoward going + // on here. + auto* expr = std::get(*i); + + ExprAttrs *nested = new ExprAttrs; + attrs->dynamicAttrs.push_back(ExprAttrs::DynamicAttrDef(expr, nested, pos)); + attrs = nested; } } // Expr insertion. // ========================== - if (i->symbol.set()) { - ExprAttrs::AttrDefs::iterator j = attrs->attrs.find(i->symbol); + if (auto* sym = std::get_if(&(*i)); sym && sym->set()) { + ExprAttrs::AttrDefs::iterator j = attrs->attrs.find(*sym); if (j != attrs->attrs.end()) { // This attr path is already defined. However, if both // e and the expr pointed by the attr path are two attribute sets, @@ -118,8 +128,9 @@ static void addAttr(ExprAttrs * attrs, AttrPath & attrPath, if (jAttrs && ae) { for (auto & ad : ae->attrs) { auto j2 = jAttrs->attrs.find(ad.first); - if (j2 != jAttrs->attrs.end()) // Attr already defined in iAttrs, error. + if (j2 != jAttrs->attrs.end()) {// Attr already defined in iAttrs, error. dupAttr(ad.first, j2->second.pos, ad.second.pos); + } jAttrs->attrs[ad.first] = ad.second; } } else { @@ -127,11 +138,13 @@ static void addAttr(ExprAttrs * attrs, AttrPath & attrPath, } } else { // This attr path is not defined. Let's create it. - attrs->attrs[i->symbol] = ExprAttrs::AttrDef(e, pos); - e->setName(i->symbol); + attrs->attrs[*sym] = ExprAttrs::AttrDef(e, pos); + e->setName(*sym); } } else { - attrs->dynamicAttrs.push_back(ExprAttrs::DynamicAttrDef(i->expr, e, pos)); + // Same caveat as the identical line above. + auto* expr = std::get(*i); + attrs->dynamicAttrs.push_back(ExprAttrs::DynamicAttrDef(expr, e, pos)); } } @@ -444,19 +457,23 @@ binds | binds INHERIT attrs ';' { $$ = $1; for (auto & i : *$3) { - if ($$->attrs.find(i.symbol) != $$->attrs.end()) - dupAttr(i.symbol, makeCurPos(@3, data), $$->attrs[i.symbol].pos); + auto sym = std::get(i); + if ($$->attrs.find(sym) != $$->attrs.end()) { + dupAttr(sym, makeCurPos(@3, data), $$->attrs[sym].pos); + } Pos pos = makeCurPos(@3, data); - $$->attrs[i.symbol] = ExprAttrs::AttrDef(new ExprVar(CUR_POS, i.symbol), pos, true); + $$->attrs[sym] = ExprAttrs::AttrDef(new ExprVar(CUR_POS, sym), pos, true); } } | binds INHERIT '(' expr ')' attrs ';' { $$ = $1; /* !!! Should ensure sharing of the expression in $4. */ for (auto & i : *$6) { - if ($$->attrs.find(i.symbol) != $$->attrs.end()) - dupAttr(i.symbol, makeCurPos(@6, data), $$->attrs[i.symbol].pos); - $$->attrs[i.symbol] = ExprAttrs::AttrDef(new ExprSelect(CUR_POS, $4, i.symbol), makeCurPos(@6, data)); + auto sym = std::get(i); + if ($$->attrs.find(sym) != $$->attrs.end()) { + dupAttr(sym, makeCurPos(@6, data), $$->attrs[sym].pos); + } + $$->attrs[sym] = ExprAttrs::AttrDef(new ExprSelect(CUR_POS, $4, sym), makeCurPos(@6, data)); } } | { $$ = new ExprAttrs; } diff --git a/third_party/nix/src/libexpr/symbol-table.hh b/third_party/nix/src/libexpr/symbol-table.hh index c4c1163be..efc2314b7 100644 --- a/third_party/nix/src/libexpr/symbol-table.hh +++ b/third_party/nix/src/libexpr/symbol-table.hh @@ -21,7 +21,7 @@ class Symbol { bool operator<(const Symbol& s2) const { return *s < *s2.s; } - operator const std::string&() const { return *s; } + operator const std::string &() const { return *s; } bool set() const { return s; } diff --git a/third_party/nix/src/libutil/config.cc b/third_party/nix/src/libutil/config.cc index 20c70c823..5d28b7a20 100644 --- a/third_party/nix/src/libutil/config.cc +++ b/third_party/nix/src/libutil/config.cc @@ -102,8 +102,8 @@ void AbstractConfig::applyConfigFile(const Path& path) { } // TODO(tazjin): absl::string_view after path functions are fixed. - std::vector tokens = - absl::StrSplit(line, absl::ByAnyChar(" \t\n\r"), absl::SkipWhitespace()); + std::vector tokens = absl::StrSplit( + line, absl::ByAnyChar(" \t\n\r"), absl::SkipWhitespace()); if (tokens.empty()) { continue; } diff --git a/third_party/nix/src/nix-daemon/nix-daemon.cc b/third_party/nix/src/nix-daemon/nix-daemon.cc index c7967307c..da3d909f1 100644 --- a/third_party/nix/src/nix-daemon/nix-daemon.cc +++ b/third_party/nix/src/nix-daemon/nix-daemon.cc @@ -16,6 +16,7 @@ #include #include "libmain/shared.hh" +#include "libproto/worker.pb.h" #include "libstore/derivations.hh" #include "libstore/globals.hh" #include "libstore/local-store.hh" @@ -27,7 +28,6 @@ #include "libutil/serialise.hh" #include "libutil/util.hh" #include "nix/legacy.hh" -#include "libproto/worker.pb.h" using namespace nix;