refactor(3p/nix/libexpr): Make nix::AttrName a std::variant
nix:AttrName was one of the few classes that relied on the default constructor of nix::Symbol (which I am trying to remove in a separate change). The class essentially represents the name of an attribute in a set, which is either just a string expression or a dynamically evaluated expression (e.g. string interpolation). Previously it would be constructed by only setting one of the fields and defaulting the other, now it is an explicit std::variant. Note that there are several code paths where not all eventualities are handled and this code is bug-for-bug compatible with those, except that unknown conditions (which should never work) are now throwing instead of silently doing ... something. The language tests pass with this change, and the depot derivations that I tested with evaluated successfully. Change-Id: Icf1ee60a5f8308f4ab18a82749e00cf37a938a8f Reviewed-on: https://cl.tvl.fyi/c/depot/+/1138 Reviewed-by: edef <edef@edef.eu> Reviewed-by: glittershark <grfn@gws.fyi> Tested-by: BuildkiteCI
This commit is contained in:
parent
693661fe87
commit
f3165f48aa
7 changed files with 67 additions and 53 deletions
25
third_party/nix/src/libexpr/eval.cc
vendored
25
third_party/nix/src/libexpr/eval.cc
vendored
|
@ -6,6 +6,7 @@
|
||||||
#include <fstream>
|
#include <fstream>
|
||||||
#include <iostream>
|
#include <iostream>
|
||||||
#include <new>
|
#include <new>
|
||||||
|
#include <variant>
|
||||||
|
|
||||||
#include <absl/strings/match.h>
|
#include <absl/strings/match.h>
|
||||||
#include <gc/gc.h>
|
#include <gc/gc.h>
|
||||||
|
@ -24,6 +25,7 @@
|
||||||
#include "libutil/hash.hh"
|
#include "libutil/hash.hh"
|
||||||
#include "libutil/json.hh"
|
#include "libutil/json.hh"
|
||||||
#include "libutil/util.hh"
|
#include "libutil/util.hh"
|
||||||
|
#include "libutil/visitor.hh"
|
||||||
|
|
||||||
namespace nix {
|
namespace nix {
|
||||||
|
|
||||||
|
@ -185,13 +187,15 @@ static void* oomHandler(size_t requested) {
|
||||||
#endif
|
#endif
|
||||||
|
|
||||||
static Symbol getName(const AttrName& name, EvalState& state, Env& env) {
|
static Symbol getName(const AttrName& name, EvalState& state, Env& env) {
|
||||||
if (name.symbol.set()) {
|
return std::visit(
|
||||||
return name.symbol;
|
util::overloaded{[&](const Symbol& name) -> Symbol { return name; },
|
||||||
}
|
[&](Expr* expr) -> Symbol {
|
||||||
Value nameValue;
|
Value nameValue;
|
||||||
name.expr->eval(state, env, nameValue);
|
expr->eval(state, env, nameValue);
|
||||||
state.forceStringNoCtx(nameValue);
|
state.forceStringNoCtx(nameValue);
|
||||||
return state.symbols.Create(nameValue.string.s);
|
return state.symbols.Create(nameValue.string.s);
|
||||||
|
}},
|
||||||
|
name);
|
||||||
}
|
}
|
||||||
|
|
||||||
static bool gcInitialised = false;
|
static bool gcInitialised = false;
|
||||||
|
@ -890,12 +894,7 @@ static std::string showAttrPath(EvalState& state, Env& env,
|
||||||
} else {
|
} else {
|
||||||
first = false;
|
first = false;
|
||||||
}
|
}
|
||||||
try {
|
out << getName(i, state, env);
|
||||||
out << getName(i, state, env);
|
|
||||||
} catch (Error& e) {
|
|
||||||
assert(!i.symbol.set());
|
|
||||||
out << "\"${" << *i.expr << "}\"";
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
return out.str();
|
return out.str();
|
||||||
}
|
}
|
||||||
|
|
22
third_party/nix/src/libexpr/nixexpr.cc
vendored
22
third_party/nix/src/libexpr/nixexpr.cc
vendored
|
@ -1,9 +1,11 @@
|
||||||
#include "libexpr/nixexpr.hh"
|
#include "libexpr/nixexpr.hh"
|
||||||
|
|
||||||
#include <cstdlib>
|
#include <cstdlib>
|
||||||
|
#include <variant>
|
||||||
|
|
||||||
#include "libstore/derivations.hh"
|
#include "libstore/derivations.hh"
|
||||||
#include "libutil/util.hh"
|
#include "libutil/util.hh"
|
||||||
|
#include "libutil/visitor.hh"
|
||||||
|
|
||||||
namespace nix {
|
namespace nix {
|
||||||
|
|
||||||
|
@ -198,17 +200,17 @@ std::ostream& operator<<(std::ostream& str, const Pos& pos) {
|
||||||
std::string showAttrPath(const AttrPath& attrPath) {
|
std::string showAttrPath(const AttrPath& attrPath) {
|
||||||
std::ostringstream out;
|
std::ostringstream out;
|
||||||
bool first = true;
|
bool first = true;
|
||||||
for (auto& i : attrPath) {
|
for (auto& attr : attrPath) {
|
||||||
if (!first) {
|
if (!first) {
|
||||||
out << '.';
|
out << '.';
|
||||||
} else {
|
} else {
|
||||||
first = false;
|
first = false;
|
||||||
}
|
}
|
||||||
if (i.symbol.set()) {
|
|
||||||
out << i.symbol;
|
std::visit(util::overloaded{
|
||||||
} else {
|
[&](const Symbol& sym) { out << sym; },
|
||||||
out << "\"${" << *i.expr << "}\"";
|
[&](const Expr* expr) { out << "\"${" << *expr << "}\""; }},
|
||||||
}
|
attr);
|
||||||
}
|
}
|
||||||
return out.str();
|
return out.str();
|
||||||
}
|
}
|
||||||
|
@ -268,8 +270,8 @@ void ExprSelect::bindVars(const StaticEnv& env) {
|
||||||
def->bindVars(env);
|
def->bindVars(env);
|
||||||
}
|
}
|
||||||
for (auto& i : attrPath) {
|
for (auto& i : attrPath) {
|
||||||
if (!i.symbol.set()) {
|
if (auto* expr = std::get_if<Expr*>(&i)) {
|
||||||
i.expr->bindVars(env);
|
(*expr)->bindVars(env);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
@ -277,8 +279,8 @@ void ExprSelect::bindVars(const StaticEnv& env) {
|
||||||
void ExprOpHasAttr::bindVars(const StaticEnv& env) {
|
void ExprOpHasAttr::bindVars(const StaticEnv& env) {
|
||||||
e->bindVars(env);
|
e->bindVars(env);
|
||||||
for (auto& i : attrPath) {
|
for (auto& i : attrPath) {
|
||||||
if (!i.symbol.set()) {
|
if (auto* expr = std::get_if<Expr*>(&i)) {
|
||||||
i.expr->bindVars(env);
|
(*expr)->bindVars(env);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
8
third_party/nix/src/libexpr/nixexpr.hh
vendored
8
third_party/nix/src/libexpr/nixexpr.hh
vendored
|
@ -1,6 +1,7 @@
|
||||||
#pragma once
|
#pragma once
|
||||||
|
|
||||||
#include <map>
|
#include <map>
|
||||||
|
#include <variant>
|
||||||
|
|
||||||
#include "libexpr/symbol-table.hh"
|
#include "libexpr/symbol-table.hh"
|
||||||
#include "libexpr/value.hh"
|
#include "libexpr/value.hh"
|
||||||
|
@ -60,12 +61,7 @@ class EvalState;
|
||||||
struct StaticEnv;
|
struct StaticEnv;
|
||||||
|
|
||||||
/* An attribute path is a sequence of attribute names. */
|
/* An attribute path is a sequence of attribute names. */
|
||||||
struct AttrName {
|
using AttrName = std::variant<Symbol, Expr*>;
|
||||||
Symbol symbol;
|
|
||||||
Expr* expr;
|
|
||||||
AttrName(const Symbol& s) : symbol(s){};
|
|
||||||
AttrName(Expr* e) : expr(e){};
|
|
||||||
};
|
|
||||||
|
|
||||||
typedef std::vector<AttrName> AttrPath;
|
typedef std::vector<AttrName> AttrPath;
|
||||||
|
|
||||||
|
|
57
third_party/nix/src/libexpr/parser.y
vendored
57
third_party/nix/src/libexpr/parser.y
vendored
|
@ -16,6 +16,7 @@
|
||||||
#ifndef BISON_HEADER
|
#ifndef BISON_HEADER
|
||||||
#define BISON_HEADER
|
#define BISON_HEADER
|
||||||
|
|
||||||
|
#include <variant>
|
||||||
#include "libutil/util.hh"
|
#include "libutil/util.hh"
|
||||||
#include "libexpr/nixexpr.hh"
|
#include "libexpr/nixexpr.hh"
|
||||||
#include "libexpr/eval.hh"
|
#include "libexpr/eval.hh"
|
||||||
|
@ -84,30 +85,39 @@ static void addAttr(ExprAttrs * attrs, AttrPath & attrPath,
|
||||||
// Checking attrPath validity.
|
// Checking attrPath validity.
|
||||||
// ===========================
|
// ===========================
|
||||||
for (i = attrPath.begin(); i + 1 < attrPath.end(); i++) {
|
for (i = attrPath.begin(); i + 1 < attrPath.end(); i++) {
|
||||||
if (i->symbol.set()) {
|
if (const auto* sym = std::get_if<Symbol>(&(*i)); sym && sym->set()) {
|
||||||
ExprAttrs::AttrDefs::iterator j = attrs->attrs.find(i->symbol);
|
ExprAttrs::AttrDefs::iterator j = attrs->attrs.find(*sym);
|
||||||
if (j != attrs->attrs.end()) {
|
if (j != attrs->attrs.end()) {
|
||||||
if (!j->second.inherited) {
|
if (!j->second.inherited) {
|
||||||
ExprAttrs * attrs2 = dynamic_cast<ExprAttrs *>(j->second.e);
|
ExprAttrs * attrs2 = dynamic_cast<ExprAttrs *>(j->second.e);
|
||||||
if (!attrs2) { dupAttr(attrPath, pos, j->second.pos); }
|
if (!attrs2) { dupAttr(attrPath, pos, j->second.pos); }
|
||||||
attrs = attrs2;
|
attrs = attrs2;
|
||||||
} else
|
} else {
|
||||||
dupAttr(attrPath, pos, j->second.pos);
|
dupAttr(attrPath, pos, j->second.pos);
|
||||||
|
}
|
||||||
} else {
|
} else {
|
||||||
ExprAttrs * nested = new ExprAttrs;
|
ExprAttrs* nested = new ExprAttrs;
|
||||||
attrs->attrs[i->symbol] = ExprAttrs::AttrDef(nested, pos);
|
attrs->attrs[*sym] = ExprAttrs::AttrDef(nested, pos);
|
||||||
attrs = nested;
|
attrs = nested;
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
ExprAttrs *nested = new ExprAttrs;
|
// Yes, this code does not handle all conditions
|
||||||
attrs->dynamicAttrs.push_back(ExprAttrs::DynamicAttrDef(i->expr, nested, pos));
|
// exhaustively. We use std::get to throw if the condition
|
||||||
attrs = nested;
|
// 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<Expr*>(*i);
|
||||||
|
|
||||||
|
ExprAttrs *nested = new ExprAttrs;
|
||||||
|
attrs->dynamicAttrs.push_back(ExprAttrs::DynamicAttrDef(expr, nested, pos));
|
||||||
|
attrs = nested;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
// Expr insertion.
|
// Expr insertion.
|
||||||
// ==========================
|
// ==========================
|
||||||
if (i->symbol.set()) {
|
if (auto* sym = std::get_if<Symbol>(&(*i)); sym && sym->set()) {
|
||||||
ExprAttrs::AttrDefs::iterator j = attrs->attrs.find(i->symbol);
|
ExprAttrs::AttrDefs::iterator j = attrs->attrs.find(*sym);
|
||||||
if (j != attrs->attrs.end()) {
|
if (j != attrs->attrs.end()) {
|
||||||
// This attr path is already defined. However, if both
|
// This attr path is already defined. However, if both
|
||||||
// e and the expr pointed by the attr path are two attribute sets,
|
// 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) {
|
if (jAttrs && ae) {
|
||||||
for (auto & ad : ae->attrs) {
|
for (auto & ad : ae->attrs) {
|
||||||
auto j2 = jAttrs->attrs.find(ad.first);
|
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);
|
dupAttr(ad.first, j2->second.pos, ad.second.pos);
|
||||||
|
}
|
||||||
jAttrs->attrs[ad.first] = ad.second;
|
jAttrs->attrs[ad.first] = ad.second;
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
|
@ -127,11 +138,13 @@ static void addAttr(ExprAttrs * attrs, AttrPath & attrPath,
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
// This attr path is not defined. Let's create it.
|
// This attr path is not defined. Let's create it.
|
||||||
attrs->attrs[i->symbol] = ExprAttrs::AttrDef(e, pos);
|
attrs->attrs[*sym] = ExprAttrs::AttrDef(e, pos);
|
||||||
e->setName(i->symbol);
|
e->setName(*sym);
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
attrs->dynamicAttrs.push_back(ExprAttrs::DynamicAttrDef(i->expr, e, pos));
|
// Same caveat as the identical line above.
|
||||||
|
auto* expr = std::get<Expr*>(*i);
|
||||||
|
attrs->dynamicAttrs.push_back(ExprAttrs::DynamicAttrDef(expr, e, pos));
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -444,19 +457,23 @@ binds
|
||||||
| binds INHERIT attrs ';'
|
| binds INHERIT attrs ';'
|
||||||
{ $$ = $1;
|
{ $$ = $1;
|
||||||
for (auto & i : *$3) {
|
for (auto & i : *$3) {
|
||||||
if ($$->attrs.find(i.symbol) != $$->attrs.end())
|
auto sym = std::get<Symbol>(i);
|
||||||
dupAttr(i.symbol, makeCurPos(@3, data), $$->attrs[i.symbol].pos);
|
if ($$->attrs.find(sym) != $$->attrs.end()) {
|
||||||
|
dupAttr(sym, makeCurPos(@3, data), $$->attrs[sym].pos);
|
||||||
|
}
|
||||||
Pos pos = makeCurPos(@3, data);
|
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 ';'
|
| binds INHERIT '(' expr ')' attrs ';'
|
||||||
{ $$ = $1;
|
{ $$ = $1;
|
||||||
/* !!! Should ensure sharing of the expression in $4. */
|
/* !!! Should ensure sharing of the expression in $4. */
|
||||||
for (auto & i : *$6) {
|
for (auto & i : *$6) {
|
||||||
if ($$->attrs.find(i.symbol) != $$->attrs.end())
|
auto sym = std::get<Symbol>(i);
|
||||||
dupAttr(i.symbol, makeCurPos(@6, data), $$->attrs[i.symbol].pos);
|
if ($$->attrs.find(sym) != $$->attrs.end()) {
|
||||||
$$->attrs[i.symbol] = ExprAttrs::AttrDef(new ExprSelect(CUR_POS, $4, i.symbol), makeCurPos(@6, data));
|
dupAttr(sym, makeCurPos(@6, data), $$->attrs[sym].pos);
|
||||||
|
}
|
||||||
|
$$->attrs[sym] = ExprAttrs::AttrDef(new ExprSelect(CUR_POS, $4, sym), makeCurPos(@6, data));
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
| { $$ = new ExprAttrs; }
|
| { $$ = new ExprAttrs; }
|
||||||
|
|
2
third_party/nix/src/libexpr/symbol-table.hh
vendored
2
third_party/nix/src/libexpr/symbol-table.hh
vendored
|
@ -21,7 +21,7 @@ class Symbol {
|
||||||
|
|
||||||
bool operator<(const Symbol& s2) const { return *s < *s2.s; }
|
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; }
|
bool set() const { return s; }
|
||||||
|
|
||||||
|
|
4
third_party/nix/src/libutil/config.cc
vendored
4
third_party/nix/src/libutil/config.cc
vendored
|
@ -102,8 +102,8 @@ void AbstractConfig::applyConfigFile(const Path& path) {
|
||||||
}
|
}
|
||||||
|
|
||||||
// TODO(tazjin): absl::string_view after path functions are fixed.
|
// TODO(tazjin): absl::string_view after path functions are fixed.
|
||||||
std::vector<std::string> tokens =
|
std::vector<std::string> tokens = absl::StrSplit(
|
||||||
absl::StrSplit(line, absl::ByAnyChar(" \t\n\r"), absl::SkipWhitespace());
|
line, absl::ByAnyChar(" \t\n\r"), absl::SkipWhitespace());
|
||||||
if (tokens.empty()) {
|
if (tokens.empty()) {
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
|
|
2
third_party/nix/src/nix-daemon/nix-daemon.cc
vendored
2
third_party/nix/src/nix-daemon/nix-daemon.cc
vendored
|
@ -16,6 +16,7 @@
|
||||||
#include <unistd.h>
|
#include <unistd.h>
|
||||||
|
|
||||||
#include "libmain/shared.hh"
|
#include "libmain/shared.hh"
|
||||||
|
#include "libproto/worker.pb.h"
|
||||||
#include "libstore/derivations.hh"
|
#include "libstore/derivations.hh"
|
||||||
#include "libstore/globals.hh"
|
#include "libstore/globals.hh"
|
||||||
#include "libstore/local-store.hh"
|
#include "libstore/local-store.hh"
|
||||||
|
@ -27,7 +28,6 @@
|
||||||
#include "libutil/serialise.hh"
|
#include "libutil/serialise.hh"
|
||||||
#include "libutil/util.hh"
|
#include "libutil/util.hh"
|
||||||
#include "nix/legacy.hh"
|
#include "nix/legacy.hh"
|
||||||
#include "libproto/worker.pb.h"
|
|
||||||
|
|
||||||
using namespace nix;
|
using namespace nix;
|
||||||
|
|
||||||
|
|
Loading…
Reference in a new issue