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

rename SymbolIdx -> Symbol, Symbol -> SymbolStr

Browse source 
after #6218 `Symbol` no longer confers a uniqueness invariant on the
string it wraps, it is now possible to create multiple symbols that
compare equal but whose string contents have different addresses. this
guarantee is now only provided by `SymbolIdx`, leaving `Symbol` only as
a string wrapper that knows about the intricacies of how symbols need to
be formatted for output.

this change renames `SymbolIdx` to `Symbol` to restore the previous
semantics of `Symbol` to that name. we also keep the wrapper type and
rename it to `SymbolStr` instead of returning plain strings from lookups
into the symbol table because symbols are formatted for output in many
places. theoretically we do not need `SymbolStr`, only a function that
formats a string for output as a symbol, but having to wrap every symbol
that appears in a message into eg `formatSymbol()` is error-prone and
inconvient.
This commit is contained in:
pennae 2022-04-22 21:45:39 +02:00
parent 7f814d6d9a
commit a385e51a08
18 changed files with 171 additions and 159 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
src/libcmd/installables.cc
View file

@ -291,7 +291,7 @@ void completeFlakeRefWithFragment(
std::string lastAttr;
if (!attrPath.empty() && !hasSuffix(attrPathS, ".")) {
lastAttr = attrPath.back();
lastAttr = evalState->symbols[attrPath.back()];
attrPath.pop_back();
}
@ -299,11 +299,11 @@ void completeFlakeRefWithFragment(
if (!attr) continue;
for (auto & attr2 : (*attr)->getAttrs()) {
if (hasPrefix(attr2, lastAttr)) {
if (hasPrefix(evalState->symbols[attr2], lastAttr)) {
auto attrPath2 = (*attr)->getAttrPath(attr2);
/* Strip the attrpath prefix. */
attrPath2.erase(attrPath2.begin(), attrPath2.begin() + attrPathPrefix.size());
completions->add(flakeRefS + "#" + concatStringsSep(".", attrPath2));
completions->add(flakeRefS + "#" + concatStringsSep(".", evalState->symbols.resolve(attrPath2)));
}
}
}

2
src/libexpr/attr-path.cc
View file

@ -36,7 +36,7 @@ std::vector<Symbol> parseAttrPath(EvalState & state, std::string_view s)
{
std::vector<Symbol> res;
for (auto & a : parseAttrPath(s))
res.emplace_back(a);
res.push_back(state.symbols.create(a));
return res;
}

4
src/libexpr/attr-set.cc
View file

@ -26,7 +26,7 @@ Bindings * EvalState::allocBindings(size_t capacity)
/* Create a new attribute named 'name' on an existing attribute set stored
in 'vAttrs' and return the newly allocated Value which is associated with
this attribute. */
Value * EvalState::allocAttr(Value & vAttrs, const SymbolIdx & name)
Value * EvalState::allocAttr(Value & vAttrs, const Symbol & name)
{
Value * v = allocValue();
vAttrs.attrs->push_back(Attr(name, v));
@ -40,7 +40,7 @@ Value * EvalState::allocAttr(Value & vAttrs, std::string_view name)
}
Value & BindingsBuilder::alloc(const SymbolIdx & name, PosIdx pos)
Value & BindingsBuilder::alloc(const Symbol & name, PosIdx pos)
{
auto value = state.allocValue();
bindings->push_back(Attr(name, value, pos));

12
src/libexpr/attr-set.hh
View file

@ -19,10 +19,10 @@ struct Attr
both of them are uint32 wrappers, they are next to each other
to make sure that Attr has no padding on 64 bit machines. that
way we keep Attr size at two words with no wasted space. */
SymbolIdx name;
Symbol name;
PosIdx pos;
Value * value;
Attr(SymbolIdx name, Value * value, PosIdx pos = noPos)
Attr(Symbol name, Value * value, PosIdx pos = noPos)
: name(name), pos(pos), value(value) { };
Attr() { };
bool operator < (const Attr & a) const
@ -66,7 +66,7 @@ public:
attrs[size_++] = attr;
}
iterator find(const SymbolIdx & name)
iterator find(const Symbol & name)
{
Attr key(name, 0);
iterator i = std::lower_bound(begin(), end(), key);
@ -74,7 +74,7 @@ public:
return end();
}
Attr * get(const SymbolIdx & name)
Attr * get(const Symbol & name)
{
Attr key(name, 0);
iterator i = std::lower_bound(begin(), end(), key);
@ -128,7 +128,7 @@ public:
: bindings(bindings), state(state)
{ }
void insert(SymbolIdx name, Value * value, PosIdx pos = noPos)
void insert(Symbol name, Value * value, PosIdx pos = noPos)
{
insert(Attr(name, value, pos));
}
@ -143,7 +143,7 @@ public:
bindings->push_back(attr);
}
Value & alloc(const SymbolIdx & name, PosIdx pos = noPos);
Value & alloc(const Symbol & name, PosIdx pos = noPos);
Value & alloc(std::string_view name, PosIdx pos = noPos);

46
src/libexpr/eval-cache.cc
View file

@ -92,6 +92,7 @@ struct AttrDb
AttrId setAttrs(
AttrKey key,
const SymbolTable & symbols,
const std::vector<Symbol> & attrs)
{
return doSQLite([&]()
@ -110,7 +111,7 @@ struct AttrDb
for (auto & attr : attrs)
state->insertAttribute.use()
(rowId)
(attr)
(symbols[attr])
(AttrType::Placeholder)
(0, false).exec();
@ -253,7 +254,7 @@ struct AttrDb
std::vector<Symbol> attrs;
auto queryAttributes(state->queryAttributes.use()(rowId));
while (queryAttributes.next())
attrs.emplace_back(queryAttributes.getStr(0));
attrs.emplace_back(symbols.create(queryAttributes.getStr(0)));
return {{rowId, attrs}};
}
case AttrType::String: {
@ -331,7 +332,7 @@ AttrKey AttrCursor::getKey()
parent->first->getKey(), root->state.symbols);
assert(parent->first->cachedValue);
}
return {parent->first->cachedValue->first, parent->second};
return {parent->first->cachedValue->first, root->state.symbols[parent->second]};
}
Value & AttrCursor::getValue()
@ -340,7 +341,7 @@ Value & AttrCursor::getValue()
if (parent) {
auto & vParent = parent->first->getValue();
root->state.forceAttrs(vParent, noPos);
auto attr = vParent.attrs->get(root->state.symbols.create(parent->second));
auto attr = vParent.attrs->get(parent->second);
if (!attr)
throw Error("attribute '%s' is unexpectedly missing", getAttrPathStr());
_value = allocRootValue(attr->value);
@ -369,12 +370,12 @@ std::vector<Symbol> AttrCursor::getAttrPath(Symbol name) const
std::string AttrCursor::getAttrPathStr() const
{
return concatStringsSep(".", getAttrPath());
return concatStringsSep(".", root->state.symbols.resolve(getAttrPath()));
}
std::string AttrCursor::getAttrPathStr(Symbol name) const
{
return concatStringsSep(".", getAttrPath(name));
return concatStringsSep(".", root->state.symbols.resolve(getAttrPath(name)));
}
Value & AttrCursor::forceValue()
@ -414,9 +415,9 @@ Suggestions AttrCursor::getSuggestionsForAttr(Symbol name)
auto attrNames = getAttrs();
std::set<std::string> strAttrNames;
for (auto & name : attrNames)
strAttrNames.insert(std::string(name));
strAttrNames.insert(root->state.symbols[name]);
return Suggestions::bestMatches(strAttrNames, name);
return Suggestions::bestMatches(strAttrNames, root->state.symbols[name]);
}
std::shared_ptr<AttrCursor> AttrCursor::maybeGetAttr(std::string_view name, bool forceErrors)
@ -428,11 +429,11 @@ std::shared_ptr<AttrCursor> AttrCursor::maybeGetAttr(std::string_view name, bool
if (cachedValue) {
if (auto attrs = std::get_if<std::vector<Symbol>>(&cachedValue->second)) {
for (auto & attr : *attrs)
if (attr == name)
return std::make_shared<AttrCursor>(root, std::make_pair(shared_from_this(), name));
if (root->state.symbols[attr] == name)
return std::make_shared<AttrCursor>(root, std::make_pair(shared_from_this(), attr));
return nullptr;
} else if (std::get_if<placeholder_t>(&cachedValue->second)) {
auto attr = root->db->getAttr({cachedValue->first, name}, root->state.symbols);
auto attr = root->db->getAttr({cachedValue->first, std::string(name)}, root->state.symbols);
if (attr) {
if (std::get_if<missing_t>(&attr->second))
return nullptr;
@ -440,10 +441,10 @@ std::shared_ptr<AttrCursor> AttrCursor::maybeGetAttr(std::string_view name, bool
if (forceErrors)
debug("reevaluating failed cached attribute '%s'");
else
throw CachedEvalError("cached failure of attribute '%s'", getAttrPathStr(name));
throw CachedEvalError("cached failure of attribute '%s'", getAttrPathStr(root->state.symbols.create(name)));
} else
return std::make_shared<AttrCursor>(root,
std::make_pair(shared_from_this(), name), nullptr, std::move(attr));
std::make_pair(shared_from_this(), root->state.symbols.create(name)), nullptr, std::move(attr));
}
// Incomplete attrset, so need to fall thru and
// evaluate to see whether 'name' exists
@ -470,7 +471,7 @@ std::shared_ptr<AttrCursor> AttrCursor::maybeGetAttr(std::string_view name, bool
if (root->db) {
if (!cachedValue)
cachedValue = {root->db->setPlaceholder(getKey()), placeholder_t()};
root->db->setMissing({cachedValue->first, name});
root->db->setMissing({cachedValue->first, std::string(name)});
}
return nullptr;
}
@ -479,18 +480,18 @@ std::shared_ptr<AttrCursor> AttrCursor::maybeGetAttr(std::string_view name, bool
if (root->db) {
if (!cachedValue)
cachedValue = {root->db->setPlaceholder(getKey()), placeholder_t()};
cachedValue2 = {root->db->setPlaceholder({cachedValue->first, name}), placeholder_t()};
cachedValue2 = {root->db->setPlaceholder({cachedValue->first, std::string(name)}), placeholder_t()};
}
return make_ref<AttrCursor>(
root, std::make_pair(shared_from_this(), name), attr->value, std::move(cachedValue2));
root, std::make_pair(shared_from_this(), root->state.symbols.create(name)), attr->value, std::move(cachedValue2));
}
ref<AttrCursor> AttrCursor::getAttr(std::string_view name, bool forceErrors)
{
auto p = maybeGetAttr(name, forceErrors);
if (!p)
throw Error("attribute '%s' does not exist", getAttrPathStr(name));
throw Error("attribute '%s' does not exist", getAttrPathStr(root->state.symbols.create(name)));
return ref(p);
}
@ -498,7 +499,7 @@ OrSuggestions<ref<AttrCursor>> AttrCursor::findAlongAttrPath(const std::vector<S
{
auto res = shared_from_this();
for (auto & attr : attrPath) {
auto child = res->maybeGetAttr(attr, force);
auto child = res->maybeGetAttr(root->state.symbols[attr], force);
if (!child) {
auto suggestions = res->getSuggestionsForAttr(attr);
return OrSuggestions<ref<AttrCursor>>::failed(suggestions);
@ -606,13 +607,14 @@ std::vector<Symbol> AttrCursor::getAttrs()
std::vector<Symbol> attrs;
for (auto & attr : *getValue().attrs)
attrs.push_back(root->state.symbols[attr.name]);
std::sort(attrs.begin(), attrs.end(), [](const Symbol & a, const Symbol & b) {
return (const std::string &) a < (const std::string &) b;
attrs.push_back(attr.name);
std::sort(attrs.begin(), attrs.end(), [&](Symbol a, Symbol b) {
std::string_view sa = root->state.symbols[a], sb = root->state.symbols[b];
return sa < sb;
});
if (root->db)
cachedValue = {root->db->setAttrs(getKey(), attrs), attrs};
cachedValue = {root->db->setAttrs(getKey(), root->state.symbols, attrs), attrs};
return attrs;
}

2
src/libexpr/eval-cache.hh
View file

@ -51,7 +51,7 @@ struct missing_t {};
struct misc_t {};
struct failed_t {};
typedef uint64_t AttrId;
typedef std::pair<AttrId, Symbol> AttrKey;
typedef std::pair<AttrId, std::string> AttrKey;
typedef std::pair<std::string, NixStringContext> string_t;
typedef std::variant<

8
src/libexpr/eval.cc
View file

@ -308,7 +308,7 @@ static BoehmGCStackAllocator boehmGCStackAllocator;
#endif
static SymbolIdx getName(const AttrName & name, EvalState & state, Env & env)
static Symbol getName(const AttrName & name, EvalState & state, Env & env)
{
if (name.symbol) {
return name.symbol;
@ -769,7 +769,7 @@ void EvalState::throwEvalError(const PosIdx pos, const char * s, const std::stri
});
}
void EvalState::throwEvalError(const PosIdx p1, const char * s, const SymbolIdx sym, const PosIdx p2) const
void EvalState::throwEvalError(const PosIdx p1, const char * s, const Symbol sym, const PosIdx p2) const
{
// p1 is where the error occurred; p2 is a position mentioned in the message.
throw EvalError({
@ -787,7 +787,7 @@ void EvalState::throwTypeError(const PosIdx pos, const char * s) const
}
void EvalState::throwTypeError(const PosIdx pos, const char * s, const ExprLambda & fun,
const SymbolIdx s2) const
const Symbol s2) const
{
throw TypeError({
.msg = hintfmt(s, fun.showNamePos(*this), symbols[s2]),
@ -796,7 +796,7 @@ void EvalState::throwTypeError(const PosIdx pos, const char * s, const ExprLambd
}
void EvalState::throwTypeError(const PosIdx pos, const Suggestions & suggestions, const char * s,
const ExprLambda & fun, const SymbolIdx s2) const
const ExprLambda & fun, const Symbol s2) const
{
throw TypeError(ErrorInfo {
.msg = hintfmt(s, fun.showNamePos(*this), symbols[s2]),

12
src/libexpr/eval.hh
View file

@ -78,7 +78,7 @@ public:
static inline std::string derivationNixPath = "//builtin/derivation.nix";
const SymbolIdx sWith, sOutPath, sDrvPath, sType, sMeta, sName, sValue,
const Symbol sWith, sOutPath, sDrvPath, sType, sMeta, sName, sValue,
sSystem, sOverrides, sOutputs, sOutputName, sIgnoreNulls,
sFile, sLine, sColumn, sFunctor, sToString,
sRight, sWrong, sStructuredAttrs, sBuilder, sArgs,
@ -87,7 +87,7 @@ public:
sRecurseForDerivations,
sDescription, sSelf, sEpsilon, sStartSet, sOperator, sKey, sPath,
sPrefix;
SymbolIdx sDerivationNix;
Symbol sDerivationNix;
/* If set, force copying files to the Nix store even if they
already exist there. */
@ -269,14 +269,14 @@ public:
[[gnu::noinline, gnu::noreturn]]
void throwEvalError(const PosIdx pos, const char * s, const std::string & s2, const std::string & s3) const;
[[gnu::noinline, gnu::noreturn]]
void throwEvalError(const PosIdx p1, const char * s, const SymbolIdx sym, const PosIdx p2) const;
void throwEvalError(const PosIdx p1, const char * s, const Symbol sym, const PosIdx p2) const;
[[gnu::noinline, gnu::noreturn]]
void throwTypeError(const PosIdx pos, const char * s) const;
[[gnu::noinline, gnu::noreturn]]
void throwTypeError(const PosIdx pos, const char * s, const ExprLambda & fun, const SymbolIdx s2) const;
void throwTypeError(const PosIdx pos, const char * s, const ExprLambda & fun, const Symbol s2) const;
[[gnu::noinline, gnu::noreturn]]
void throwTypeError(const PosIdx pos, const Suggestions & suggestions, const char * s,
const ExprLambda & fun, const SymbolIdx s2) const;
const ExprLambda & fun, const Symbol s2) const;
[[gnu::noinline, gnu::noreturn]]
void throwTypeError(const char * s, const Value & v) const;
[[gnu::noinline, gnu::noreturn]]
@ -392,7 +392,7 @@ public:
inline Value * allocValue();
inline Env & allocEnv(size_t size);
Value * allocAttr(Value & vAttrs, const SymbolIdx & name);
Value * allocAttr(Value & vAttrs, const Symbol & name);
Value * allocAttr(Value & vAttrs, std::string_view name);
Bindings * allocBindings(size_t capacity);

19
src/libexpr/nixexpr.cc
View file

@ -24,8 +24,10 @@ static void showString(std::ostream & str, std::string_view s)
str << '"';
}
static void showId(std::ostream & str, std::string_view s)
std::ostream & operator <<(std::ostream & str, const SymbolStr & symbol)
{
std::string_view s = symbol;
if (s.empty())
str << "\"\"";
else if (s == "if") // FIXME: handle other keywords
@ -34,7 +36,7 @@ static void showId(std::ostream & str, std::string_view s)
char c = s[0];
if (!((c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || c == '_')) {
showString(str, s);
return;
return str;
}
for (auto c : s)
if (!((c >= 'a' && c <= 'z') ||
@ -42,15 +44,10 @@ static void showId(std::ostream & str, std::string_view s)
(c >= '0' && c <= '9') ||
c == '_' || c == '\'' || c == '-')) {
showString(str, s);
return;
return str;
}
str << s;
}
}
std::ostream & operator << (std::ostream & str, const Symbol & sym)
{
showId(str, sym.s);
return str;
}
@ -499,12 +496,12 @@ void ExprPos::bindVars(const EvalState & es, const StaticEnv & env)
/* Storing function names. */
void Expr::setName(SymbolIdx name)
void Expr::setName(Symbol name)
{
}
void ExprLambda::setName(SymbolIdx name)
void ExprLambda::setName(Symbol name)
{
this->name = name;
body->setName(name);
@ -526,7 +523,7 @@ std::string ExprLambda::showNamePos(const EvalState & state) const
size_t SymbolTable::totalSize() const
{
size_t n = 0;
dump([&] (const Symbol & s) { n += std::string_view(s).size(); });
dump([&] (const std::string & s) { n += s.size(); });
return n;
}

34
src/libexpr/nixexpr.hh
View file

@ -126,9 +126,9 @@ struct StaticEnv;
/* An attribute path is a sequence of attribute names. */
struct AttrName
{
SymbolIdx symbol;
Symbol symbol;
Expr * expr;
AttrName(const SymbolIdx & s) : symbol(s) {};
AttrName(const Symbol & s) : symbol(s) {};
AttrName(Expr * e) : expr(e) {};
};
@ -146,7 +146,7 @@ struct Expr
virtual void bindVars(const EvalState & es, const StaticEnv & env);
virtual void eval(EvalState & state, Env & env, Value & v);
virtual Value * maybeThunk(EvalState & state, Env & env);
virtual void setName(SymbolIdx name);
virtual void setName(Symbol name);
};
#define COMMON_METHODS \
@ -196,7 +196,7 @@ typedef uint32_t Displacement;
struct ExprVar : Expr
{
PosIdx pos;
SymbolIdx name;
Symbol name;
/* Whether the variable comes from an environment (e.g. a rec, let
or function argument) or from a "with". */
@ -211,8 +211,8 @@ struct ExprVar : Expr
Level level;
Displacement displ;
ExprVar(const SymbolIdx & name) : name(name) { };
ExprVar(const PosIdx & pos, const SymbolIdx & name) : pos(pos), name(name) { };
ExprVar(const Symbol & name) : name(name) { };
ExprVar(const PosIdx & pos, const Symbol & name) : pos(pos), name(name) { };
COMMON_METHODS
Value * maybeThunk(EvalState & state, Env & env);
};
@ -223,7 +223,7 @@ struct ExprSelect : Expr
Expr * e, * def;
AttrPath attrPath;
ExprSelect(const PosIdx & pos, Expr * e, const AttrPath & attrPath, Expr * def) : pos(pos), e(e), def(def), attrPath(attrPath) { };
ExprSelect(const PosIdx & pos, Expr * e, const SymbolIdx & name) : pos(pos), e(e), def(0) { attrPath.push_back(AttrName(name)); };
ExprSelect(const PosIdx & pos, Expr * e, const Symbol & name) : pos(pos), e(e), def(0) { attrPath.push_back(AttrName(name)); };
COMMON_METHODS
};
@ -248,7 +248,7 @@ struct ExprAttrs : Expr
: inherited(inherited), e(e), pos(pos) { };
AttrDef() { };
};
typedef std::map<SymbolIdx, AttrDef> AttrDefs;
typedef std::map<Symbol, AttrDef> AttrDefs;
AttrDefs attrs;
struct DynamicAttrDef {
Expr * nameExpr, * valueExpr;
@ -273,7 +273,7 @@ struct ExprList : Expr
struct Formal
{
PosIdx pos;
SymbolIdx name;
Symbol name;
Expr * def;
};
@ -283,9 +283,9 @@ struct Formals
Formals_ formals;
bool ellipsis;
bool has(SymbolIdx arg) const {
bool has(Symbol arg) const {
auto it = std::lower_bound(formals.begin(), formals.end(), arg,
[] (const Formal & f, const SymbolIdx & sym) { return f.name < sym; });
[] (const Formal & f, const Symbol & sym) { return f.name < sym; });
return it != formals.end() && it->name == arg;
}
@ -304,11 +304,11 @@ struct Formals
struct ExprLambda : Expr
{
PosIdx pos;
SymbolIdx name;
SymbolIdx arg;
Symbol name;
Symbol arg;
Formals * formals;
Expr * body;
ExprLambda(PosIdx pos, SymbolIdx arg, Formals * formals, Expr * body)
ExprLambda(PosIdx pos, Symbol arg, Formals * formals, Expr * body)
: pos(pos), arg(arg), formals(formals), body(body)
{
};
@ -316,7 +316,7 @@ struct ExprLambda : Expr
: pos(pos), formals(formals), body(body)
{
}
void setName(SymbolIdx name);
void setName(Symbol name);
std::string showNamePos(const EvalState & state) const;
inline bool hasFormals() const { return formals != nullptr; }
COMMON_METHODS
@ -426,7 +426,7 @@ struct StaticEnv
const StaticEnv * up;
// Note: these must be in sorted order.
typedef std::vector<std::pair<SymbolIdx, Displacement>> Vars;
typedef std::vector<std::pair<Symbol, Displacement>> Vars;
Vars vars;
StaticEnv(bool isWith, const StaticEnv * up, size_t expectedSize = 0) : isWith(isWith), up(up) {
@ -450,7 +450,7 @@ struct StaticEnv
vars.erase(it, end);
}
Vars::const_iterator find(const SymbolIdx & name) const
Vars::const_iterator find(const Symbol & name) const
{
Vars::value_type key(name, 0);
auto i = std::lower_bound(vars.begin(), vars.end(), key);

6
src/libexpr/parser.y
View file

@ -86,7 +86,7 @@ static void dupAttr(const EvalState & state, const AttrPath & attrPath, const Po
});
}
static void dupAttr(const EvalState & state, SymbolIdx attr, const PosIdx pos, const PosIdx prevPos)
static void dupAttr(const EvalState & state, Symbol attr, const PosIdx pos, const PosIdx prevPos)
{
throw ParseError({
.msg = hintfmt("attribute '%1%' already defined at %2%", state.symbols[attr], state.positions[prevPos]),
@ -157,14 +157,14 @@ static void addAttr(ExprAttrs * attrs, AttrPath & attrPath,
static Formals * toFormals(ParseData & data, ParserFormals * formals,
PosIdx pos = noPos, SymbolIdx arg = {})
PosIdx pos = noPos, Symbol arg = {})
{
std::sort(formals->formals.begin(), formals->formals.end(),
[] (const auto & a, const auto & b) {
return std::tie(a.name, a.pos) < std::tie(b.name, b.pos);
});
std::optional<std::pair<SymbolIdx, PosIdx>> duplicate;
std::optional<std::pair<Symbol, PosIdx>> duplicate;
for (size_t i = 0; i + 1 < formals->formals.size(); i++) {
if (formals->formals[i].name != formals->formals[i + 1].name)
continue;

8
src/libexpr/primops.cc
View file

@ -584,7 +584,7 @@ typedef std::list<Value *> ValueList;
static Bindings::iterator getAttr(
EvalState & state,
std::string_view funcName,
SymbolIdx attrSym,
Symbol attrSym,
Bindings * attrSet,
const PosIdx pos)
{
@ -2047,7 +2047,7 @@ static void prim_path(EvalState & state, const PosIdx pos, Value * * args, Value
PathSet context;
for (auto & attr : *args[0]->attrs) {
auto & n(state.symbols[attr.name]);
auto n = state.symbols[attr.name];
if (n == "path")
path = state.coerceToPath(attr.pos, *attr.value, context);
else if (attr.name == state.sName)
@ -2314,7 +2314,7 @@ static void prim_listToAttrs(EvalState & state, const PosIdx pos, Value * * args
auto attrs = state.buildBindings(args[0]->listSize());
std::set<SymbolIdx> seen;
std::set<Symbol> seen;
for (auto v2 : args[0]->listItems()) {
state.forceAttrs(*v2, pos);
@ -2517,7 +2517,7 @@ static void prim_zipAttrsWith(EvalState & state, const PosIdx pos, Value * * arg
// attribute with the merge function application. this way we need not
// use (slightly slower) temporary storage the GC does not know about.
std::map<SymbolIdx, std::pair<size_t, Value * *>> attrsSeen;
std::map<Symbol, std::pair<size_t, Value * *>> attrsSeen;
state.forceFunction(*args[0], pos);
state.forceList(*args[1], pos);

60
src/libexpr/symbol-table.hh
View file

@ -12,82 +12,94 @@ namespace nix {
/* Symbol table used by the parser and evaluator to represent and look
up identifiers and attributes efficiently. SymbolTable::create()
converts a string into a symbol. Symbols have the property that
they can be compared efficiently (using a pointer equality test),
they can be compared efficiently (using an equality test),
because the symbol table stores only one copy of each string. */
class Symbol
/* This class mainly exists to give us an operator<< for ostreams. We could also
return plain strings from SymbolTable, but then we'd have to wrap every
instance of a symbol that is fmt()ed, which is inconvenient and error-prone. */
class SymbolStr
{
friend class SymbolTable;
private:
std::string s;
const std::string * s;
explicit SymbolStr(const std::string & symbol): s(&symbol) {}
public:
Symbol(std::string_view s) : s(s) { }
// FIXME: remove
bool operator == (std::string_view s2) const
{
return s == s2;
return *s == s2;
}
operator const std::string & () const
{
return s;
return *s;
}
operator const std::string_view () const
{
return s;
return *s;
}
friend std::ostream & operator << (std::ostream & str, const Symbol & sym);
friend std::ostream & operator <<(std::ostream & os, const SymbolStr & symbol);
};
class SymbolIdx
class Symbol
{
friend class SymbolTable;
private:
uint32_t id;
explicit SymbolIdx(uint32_t id): id(id) {}
explicit Symbol(uint32_t id): id(id) {}
public:
SymbolIdx() : id(0) {}
Symbol() : id(0) {}
explicit operator bool() const { return id > 0; }
bool operator<(const SymbolIdx other) const { return id < other.id; }
bool operator==(const SymbolIdx other) const { return id == other.id; }
bool operator!=(const SymbolIdx other) const { return id != other.id; }
bool operator<(const Symbol other) const { return id < other.id; }
bool operator==(const Symbol other) const { return id == other.id; }
bool operator!=(const Symbol other) const { return id != other.id; }
};
class SymbolTable
{
private:
std::unordered_map<std::string_view, std::pair<const Symbol *, uint32_t>> symbols;
ChunkedVector<Symbol, 8192> store{16};
std::unordered_map<std::string_view, std::pair<const std::string *, uint32_t>> symbols;
ChunkedVector<std::string, 8192> store{16};
public:
SymbolIdx create(std::string_view s)
Symbol create(std::string_view s)
{
// Most symbols are looked up more than once, so we trade off insertion performance
// for lookup performance.
// TODO: could probably be done more efficiently with transparent Hash and Equals
// on the original implementation using unordered_set
auto it = symbols.find(s);
if (it != symbols.end()) return SymbolIdx(it->second.second + 1);
if (it != symbols.end()) return Symbol(it->second.second + 1);
const auto & [rawSym, idx] = store.add(s);
const auto & [rawSym, idx] = store.add(std::string(s));
symbols.emplace(rawSym, std::make_pair(&rawSym, idx));
return SymbolIdx(idx + 1);
return Symbol(idx + 1);
}
const Symbol & operator[](SymbolIdx s) const
std::vector<SymbolStr> resolve(const std::vector<Symbol> & symbols) const
{
std::vector<SymbolStr> result;
result.reserve(symbols.size());
for (auto sym : symbols)
result.push_back((*this)[sym]);
return result;
}
SymbolStr operator[](Symbol s) const
{
if (s.id == 0 || s.id > store.size())
abort();
return store[s.id - 1];
return SymbolStr(store[s.id - 1]);
}
size_t size() const

15
src/libexpr/value.hh
View file

@ -55,7 +55,7 @@ struct Env;
struct Expr;
struct ExprLambda;
struct PrimOp;
class SymbolIdx;
class Symbol;
class PosIdx;
struct Pos;
class StorePath;
@ -251,11 +251,6 @@ public:
void mkStringMove(const char * s, const PathSet & context);
inline void mkString(const Symbol & s)
{
mkString(std::string_view(s).data());
}
inline void mkPath(const char * s)
{
clearValue();
@ -410,12 +405,12 @@ public:
#if HAVE_BOEHMGC
typedef std::vector<Value *, traceable_allocator<Value *> > ValueVector;
typedef std::map<SymbolIdx, Value *, std::less<SymbolIdx>, traceable_allocator<std::pair<const SymbolIdx, Value *> > > ValueMap;
typedef std::map<SymbolIdx, ValueVector, std::less<SymbolIdx>, traceable_allocator<std::pair<const SymbolIdx, ValueVector> > > ValueVectorMap;
typedef std::map<Symbol, Value *, std::less<Symbol>, traceable_allocator<std::pair<const Symbol, Value *> > > ValueMap;
typedef std::map<Symbol, ValueVector, std::less<Symbol>, traceable_allocator<std::pair<const Symbol, ValueVector> > > ValueVectorMap;
#else
typedef std::vector<Value *> ValueVector;
typedef std::map<SymbolIdx, Value *> ValueMap;
typedef std::map<SymbolIdx, ValueVector> ValueVectorMap;
typedef std::map<Symbol, Value *> ValueMap;
typedef std::map<Symbol, ValueVector> ValueVectorMap;
#endif

2
src/nix/app.cc
View file

@ -66,7 +66,7 @@ UnresolvedApp Installable::toApp(EvalState & state)
auto type = cursor->getAttr("type")->getString();
std::string expected = !attrPath.empty() && attrPath[0] == "apps" ? "app" : "derivation";
std::string expected = !attrPath.empty() && state.symbols[attrPath[0]] == "apps" ? "app" : "derivation";
if (type != expected)
throw Error("attribute '%s' should have type '%s'", cursor->getAttrPathStr(), expected);

74
src/nix/flake.cc
View file

@ -311,7 +311,7 @@ struct CmdFlakeCheck : FlakeCommand
return state->positions[p];
};
auto argHasName = [&] (SymbolIdx arg, std::string_view expected) {
auto argHasName = [&] (Symbol arg, std::string_view expected) {
std::string_view name = state->symbols[arg];
return
name == expected
@ -986,8 +986,11 @@ struct CmdFlakeShow : FlakeCommand, MixJSON
{
auto j = nlohmann::json::object();
auto attrPathS = state->symbols.resolve(attrPath);
Activity act(*logger, lvlInfo, actUnknown,
fmt("evaluating '%s'", concatStringsSep(".", attrPath)));
fmt("evaluating '%s'", concatStringsSep(".", attrPathS)));
try {
auto recurse = [&]()
{
@ -995,14 +998,15 @@ struct CmdFlakeShow : FlakeCommand, MixJSON
logger->cout("%s", headerPrefix);
auto attrs = visitor.getAttrs();
for (const auto & [i, attr] : enumerate(attrs)) {
const auto & attrName = state->symbols[attr];
bool last = i + 1 == attrs.size();
auto visitor2 = visitor.getAttr(attr);
auto visitor2 = visitor.getAttr(attrName);
auto attrPath2(attrPath);
attrPath2.push_back(attr);
auto j2 = visit(*visitor2, attrPath2,
fmt(ANSI_GREEN "%s%s" ANSI_NORMAL ANSI_BOLD "%s" ANSI_NORMAL, nextPrefix, last ? treeLast : treeConn, attr),
fmt(ANSI_GREEN "%s%s" ANSI_NORMAL ANSI_BOLD "%s" ANSI_NORMAL, nextPrefix, last ? treeLast : treeConn, attrName),
nextPrefix + (last ? treeNull : treeLine));
if (json) j.emplace(attr, std::move(j2));
if (json) j.emplace(attrName, std::move(j2));
}
};
@ -1022,10 +1026,10 @@ struct CmdFlakeShow : FlakeCommand, MixJSON
} else {
logger->cout("%s: %s '%s'",
headerPrefix,
attrPath.size() == 2 && attrPath[0] == "devShell" ? "development environment" :
attrPath.size() >= 2 && attrPath[0] == "devShells" ? "development environment" :
attrPath.size() == 3 && attrPath[0] == "checks" ? "derivation" :
attrPath.size() >= 1 && attrPath[0] == "hydraJobs" ? "derivation" :
attrPath.size() == 2 && attrPathS[0] == "devShell" ? "development environment" :
attrPath.size() >= 2 && attrPathS[0] == "devShells" ? "development environment" :
attrPath.size() == 3 && attrPathS[0] == "checks" ? "derivation" :
attrPath.size() >= 1 && attrPathS[0] == "hydraJobs" ? "derivation" :
"package",
name);
}
@ -1033,27 +1037,27 @@ struct CmdFlakeShow : FlakeCommand, MixJSON
if (attrPath.size() == 0
|| (attrPath.size() == 1 && (
attrPath[0] == "defaultPackage"
|| attrPath[0] == "devShell"
|| attrPath[0] == "formatter"
|| attrPath[0] == "nixosConfigurations"
|| attrPath[0] == "nixosModules"
|| attrPath[0] == "defaultApp"
|| attrPath[0] == "templates"
|| attrPath[0] == "overlays"))
attrPathS[0] == "defaultPackage"
|| attrPathS[0] == "devShell"
|| attrPathS[0] == "formatter"
|| attrPathS[0] == "nixosConfigurations"
|| attrPathS[0] == "nixosModules"
|| attrPathS[0] == "defaultApp"
|| attrPathS[0] == "templates"
|| attrPathS[0] == "overlays"))
|| ((attrPath.size() == 1 || attrPath.size() == 2)
&& (attrPath[0] == "checks"
|| attrPath[0] == "packages"
|| attrPath[0] == "devShells"
|| attrPath[0] == "apps"))
&& (attrPathS[0] == "checks"
|| attrPathS[0] == "packages"
|| attrPathS[0] == "devShells"
|| attrPathS[0] == "apps"))
)
{
recurse();
}
else if (
(attrPath.size() == 2 && (attrPath[0] == "defaultPackage" || attrPath[0] == "devShell" || attrPath[0] == "formatter"))
|| (attrPath.size() == 3 && (attrPath[0] == "checks" || attrPath[0] == "packages" || attrPath[0] == "devShells"))
(attrPath.size() == 2 && (attrPathS[0] == "defaultPackage" || attrPathS[0] == "devShell" || attrPathS[0] == "formatter"))
|| (attrPath.size() == 3 && (attrPathS[0] == "checks" || attrPathS[0] == "packages" || attrPathS[0] == "devShells"))
)
{
if (visitor.isDerivation())
@ -1062,14 +1066,14 @@ struct CmdFlakeShow : FlakeCommand, MixJSON
throw Error("expected a derivation");
}
else if (attrPath.size() > 0 && attrPath[0] == "hydraJobs") {
else if (attrPath.size() > 0 && attrPathS[0] == "hydraJobs") {
if (visitor.isDerivation())
showDerivation();
else
recurse();
}
else if (attrPath.size() > 0 && attrPath[0] == "legacyPackages") {
else if (attrPath.size() > 0 && attrPathS[0] == "legacyPackages") {
if (attrPath.size() == 1)
recurse();
else if (!showLegacy)
@ -1084,8 +1088,8 @@ struct CmdFlakeShow : FlakeCommand, MixJSON
}
else if (
(attrPath.size() == 2 && attrPath[0] == "defaultApp") ||
(attrPath.size() == 3 && attrPath[0] == "apps"))
(attrPath.size() == 2 && attrPathS[0] == "defaultApp") ||
(attrPath.size() == 3 && attrPathS[0] == "apps"))
{
auto aType = visitor.maybeGetAttr("type");
if (!aType || aType->getString() != "app")
@ -1098,8 +1102,8 @@ struct CmdFlakeShow : FlakeCommand, MixJSON
}
else if (
(attrPath.size() == 1 && attrPath[0] == "defaultTemplate") ||
(attrPath.size() == 2 && attrPath[0] == "templates"))
(attrPath.size() == 1 && attrPathS[0] == "defaultTemplate") ||
(attrPath.size() == 2 && attrPathS[0] == "templates"))
{
auto description = visitor.getAttr("description")->getString();
if (json) {
@ -1112,11 +1116,11 @@ struct CmdFlakeShow : FlakeCommand, MixJSON
else {
auto [type, description] =
(attrPath.size() == 1 && attrPath[0] == "overlay")
|| (attrPath.size() == 2 && attrPath[0] == "overlays") ? std::make_pair("nixpkgs-overlay", "Nixpkgs overlay") :
attrPath.size() == 2 && attrPath[0] == "nixosConfigurations" ? std::make_pair("nixos-configuration", "NixOS configuration") :
(attrPath.size() == 1 && attrPath[0] == "nixosModule")
|| (attrPath.size() == 2 && attrPath[0] == "nixosModules") ? std::make_pair("nixos-module", "NixOS module") :
(attrPath.size() == 1 && attrPathS[0] == "overlay")
|| (attrPath.size() == 2 && attrPathS[0] == "overlays") ? std::make_pair("nixpkgs-overlay", "Nixpkgs overlay") :
attrPath.size() == 2 && attrPathS[0] == "nixosConfigurations" ? std::make_pair("nixos-configuration", "NixOS configuration") :
(attrPath.size() == 1 && attrPathS[0] == "nixosModule")
|| (attrPath.size() == 2 && attrPathS[0] == "nixosModules") ? std::make_pair("nixos-module", "NixOS module") :
std::make_pair("unknown", "unknown");
if (json) {
j.emplace("type", type);
@ -1125,7 +1129,7 @@ struct CmdFlakeShow : FlakeCommand, MixJSON
}
}
} catch (EvalError & e) {
if (!(attrPath.size() > 0 && attrPath[0] == "legacyPackages"))
if (!(attrPath.size() > 0 && attrPathS[0] == "legacyPackages"))
throw;
}

4
src/nix/repl.cc
View file

@ -73,7 +73,7 @@ struct NixRepl
void initEnv();
void reloadFiles();
void addAttrsToScope(Value & attrs);
void addVarToScope(const SymbolIdx name, Value & v);
void addVarToScope(const Symbol name, Value & v);
Expr * parseString(std::string s);
void evalString(std::string s, Value & v);
@ -711,7 +711,7 @@ void NixRepl::addAttrsToScope(Value & attrs)
}
void NixRepl::addVarToScope(const SymbolIdx name, Value & v)
void NixRepl::addVarToScope(const Symbol name, Value & v)
{
if (displ >= envSize)
throw Error("environment full; cannot add more variables");

16
src/nix/search.cc
View file

@ -77,13 +77,15 @@ struct CmdSearch : InstallableCommand, MixJSON
visit = [&](eval_cache::AttrCursor & cursor, const std::vector<Symbol> & attrPath, bool initialRecurse)
{
auto attrPathS = state->symbols.resolve(attrPath);
Activity act(*logger, lvlInfo, actUnknown,
fmt("evaluating '%s'", concatStringsSep(".", attrPath)));
fmt("evaluating '%s'", concatStringsSep(".", attrPathS)));
try {
auto recurse = [&]()
{
for (const auto & attr : cursor.getAttrs()) {
auto cursor2 = cursor.getAttr(attr);
auto cursor2 = cursor.getAttr(state->symbols[attr]);
auto attrPath2(attrPath);
attrPath2.push_back(attr);
visit(*cursor2, attrPath2, false);
@ -97,7 +99,7 @@ struct CmdSearch : InstallableCommand, MixJSON
auto aDescription = aMeta ? aMeta->maybeGetAttr("description") : nullptr;
auto description = aDescription ? aDescription->getString() : "";
std::replace(description.begin(), description.end(), '\n', ' ');
auto attrPath2 = concatStringsSep(".", attrPath);
auto attrPath2 = concatStringsSep(".", attrPathS);
std::vector<std::smatch> attrPathMatches;
std::vector<std::smatch> descriptionMatches;
@ -146,21 +148,21 @@ struct CmdSearch : InstallableCommand, MixJSON
else if (
attrPath.size() == 0
|| (attrPath[0] == "legacyPackages" && attrPath.size() <= 2)
|| (attrPath[0] == "packages" && attrPath.size() <= 2))
|| (attrPathS[0] == "legacyPackages" && attrPath.size() <= 2)
|| (attrPathS[0] == "packages" && attrPath.size() <= 2))
recurse();
else if (initialRecurse)
recurse();
else if (attrPath[0] == "legacyPackages" && attrPath.size() > 2) {
else if (attrPathS[0] == "legacyPackages" && attrPath.size() > 2) {
auto attr = cursor.maybeGetAttr("recurseForDerivations");
if (attr && attr->getBool())
recurse();
}
} catch (EvalError & e) {
if (!(attrPath.size() > 0 && attrPath[0] == "legacyPackages"))
if (!(attrPath.size() > 0 && attrPathS[0] == "legacyPackages"))
throw;
}
};