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

libexpr: simplify EvalErrorBuilder interface

Browse source 
if we hold the error being built behind a pointer we no longer need to
allocate the error builder itself to avoid impacting eval performance.
adding && ref qualifiers to builder functions and adding the nodiscard
attribute to the class itself also makes the nodiscard attributes kept
on the builder functions unnecessary. we do keep the noinlines though,
removing them still regresses eval performance even after this change.

Change-Id: Ibc14a66955ac32142d97fb3680b0a7e14db250cd
This commit is contained in:
eldritch horrors 2024-12-03 20:38:41 +01:00
parent ef1d62ec6c
commit a4fa93d469
3 changed files with 44 additions and 56 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

59
lix/libexpr/eval-error.cc
View file

@ -5,45 +5,45 @@
namespace nix {
template<class T>
EvalErrorBuilder<T> & EvalErrorBuilder<T>::withExitStatus(unsigned int exitStatus)
EvalErrorBuilder<T> EvalErrorBuilder<T>::withExitStatus(unsigned int exitStatus) &&
{
error.withExitStatus(exitStatus);
return *this;
error->withExitStatus(exitStatus);
return std::move(*this);
}
template<class T>
EvalErrorBuilder<T> & EvalErrorBuilder<T>::atPos(PosIdx pos)
EvalErrorBuilder<T> EvalErrorBuilder<T>::atPos(PosIdx pos) &&
{
error.err.pos = state.positions[pos];
return *this;
error->err.pos = state.positions[pos];
return std::move(*this);
}
template<class T>
EvalErrorBuilder<T> & EvalErrorBuilder<T>::atPos(Value & value, PosIdx fallback)
EvalErrorBuilder<T> EvalErrorBuilder<T>::atPos(Value & value, PosIdx fallback) &&
{
return atPos(value.determinePos(fallback));
return std::move(*this).atPos(value.determinePos(fallback));
}
template<class T>
EvalErrorBuilder<T> & EvalErrorBuilder<T>::withTrace(PosIdx pos, const std::string_view text)
EvalErrorBuilder<T> EvalErrorBuilder<T>::withTrace(PosIdx pos, const std::string_view text) &&
{
error.err.traces.push_front(
error->err.traces.push_front(
Trace{.pos = state.positions[pos], .hint = HintFmt(std::string(text))});
return *this;
return std::move(*this);
}
template<class T>
EvalErrorBuilder<T> & EvalErrorBuilder<T>::withSuggestions(Suggestions & s)
EvalErrorBuilder<T> EvalErrorBuilder<T>::withSuggestions(Suggestions & s) &&
{
error.err.suggestions = s;
return *this;
error->err.suggestions = s;
return std::move(*this);
}
template<class T>
EvalErrorBuilder<T> & EvalErrorBuilder<T>::withFrame(const Env & env, const Expr & expr)
EvalErrorBuilder<T> EvalErrorBuilder<T>::withFrame(const Env & env, const Expr & expr) &&
{
if (state.debug) {
error.frame = state.debug->addTrace(DebugTrace{
error->frame = state.debug->addTrace(DebugTrace{
.pos = state.positions[expr.getPos()],
.expr = expr,
.env = env,
@ -51,45 +51,38 @@ EvalErrorBuilder<T> & EvalErrorBuilder<T>::withFrame(const Env & env, const Expr
.isError = true
}).entry;
}
return *this;
return std::move(*this);
}
template<class T>
EvalErrorBuilder<T> & EvalErrorBuilder<T>::addTrace(PosIdx pos, HintFmt hint)
EvalErrorBuilder<T> EvalErrorBuilder<T>::addTrace(PosIdx pos, HintFmt hint) &&
{
error.addTrace(state.positions[pos], hint);
return *this;
error->addTrace(state.positions[pos], hint);
return std::move(*this);
}
template<class T>
template<typename... Args>
EvalErrorBuilder<T> &
EvalErrorBuilder<T>::addTrace(PosIdx pos, std::string_view formatString, const Args &... formatArgs)
EvalErrorBuilder<T>
EvalErrorBuilder<T>::addTrace(PosIdx pos, std::string_view formatString, const Args &... formatArgs) &&
{
addTrace(state.positions[pos], HintFmt(std::string(formatString), formatArgs...));
return *this;
return std::move(*this);
}
template<class T>
void EvalErrorBuilder<T>::debugThrow()
void EvalErrorBuilder<T>::debugThrow() &&
{
if (state.debug) {
if (auto last = state.debug->traces().next()) {
const Env * env = &(*last)->env;
const Expr * expr = &(*last)->expr;
state.debug->onEvalError(&error, *env, *expr);
state.debug->onEvalError(error.get(), *env, *expr);
}
}
// `EvalState` is the only class that can construct an `EvalErrorBuilder`,
// and it does so in dynamic storage. This is the final method called on
// any such instance and must delete itself before throwing the underlying
// error.
auto error = std::move(this->error);
delete this;
throw error;
throw *error;
}
template class EvalErrorBuilder<EvalError>;

34
lix/libexpr/eval-error.hh
View file

@ -1,6 +1,7 @@
#pragma once
///@file
#include "lix/libutil/box_ptr.hh"
#include "lix/libutil/error.hh"
#include "lix/libutil/types.hh"
#include "lix/libexpr/pos-idx.hh"
@ -51,13 +52,8 @@ public:
}
};
/**
* `EvalErrorBuilder`s may only be constructed by `EvalState`. The `debugThrow`
* method must be the final method in any such `EvalErrorBuilder` usage, and it
* handles deleting the object.
*/
template<class T>
class EvalErrorBuilder final
class [[nodiscard]] EvalErrorBuilder final
{
friend class EvalState;
@ -65,35 +61,35 @@ class EvalErrorBuilder final
template<typename... Args>
explicit EvalErrorBuilder(EvalState & state, const Args &... args)
: state(state), error(T(args...))
: state(state), error(make_box_ptr<T>(args...))
{
}
public:
T error;
box_ptr<T> error;
[[nodiscard, gnu::noinline]] EvalErrorBuilder<T> & withExitStatus(unsigned int exitStatus);
[[gnu::noinline]] EvalErrorBuilder<T> withExitStatus(unsigned int exitStatus) &&;
[[nodiscard, gnu::noinline]] EvalErrorBuilder<T> & atPos(PosIdx pos);
[[gnu::noinline]] EvalErrorBuilder<T> atPos(PosIdx pos) &&;
[[nodiscard, gnu::noinline]] EvalErrorBuilder<T> & atPos(Value & value, PosIdx fallback = noPos);
[[gnu::noinline]] EvalErrorBuilder<T> atPos(Value & value, PosIdx fallback = noPos) &&;
[[nodiscard, gnu::noinline]] EvalErrorBuilder<T> & withTrace(PosIdx pos, const std::string_view text);
[[gnu::noinline]] EvalErrorBuilder<T> withTrace(PosIdx pos, const std::string_view text) &&;
[[nodiscard, gnu::noinline]] EvalErrorBuilder<T> & withSuggestions(Suggestions & s);
[[gnu::noinline]] EvalErrorBuilder<T> withSuggestions(Suggestions & s) &&;
[[nodiscard, gnu::noinline]] EvalErrorBuilder<T> & withFrame(const Env & e, const Expr & ex);
[[gnu::noinline]] EvalErrorBuilder<T> withFrame(const Env & e, const Expr & ex) &&;
[[nodiscard, gnu::noinline]] EvalErrorBuilder<T> & addTrace(PosIdx pos, HintFmt hint);
[[gnu::noinline]] EvalErrorBuilder<T> addTrace(PosIdx pos, HintFmt hint) &&;
template<typename... Args>
[[nodiscard, gnu::noinline]] EvalErrorBuilder<T> &
addTrace(PosIdx pos, std::string_view formatString, const Args &... formatArgs);
[[gnu::noinline]] EvalErrorBuilder<T>
addTrace(PosIdx pos, std::string_view formatString, const Args &... formatArgs) &&;
/**
* Delete the `EvalErrorBuilder` and throw the underlying exception.
* Throw the underlying exception.
*/
[[gnu::noinline, gnu::noreturn]] void debugThrow();
[[gnu::noinline, gnu::noreturn]] void debugThrow() &&;
};
}

7
lix/libexpr/eval.hh
View file

@ -391,10 +391,9 @@ public:
std::unique_ptr<DebugState> debug;
template<class T, typename... Args>
[[nodiscard, gnu::noinline]]
EvalErrorBuilder<T> & error(const Args & ... args) {
// `EvalErrorBuilder::debugThrow` performs the corresponding `delete`.
return *new EvalErrorBuilder<T>(*this, args...);
[[gnu::noinline]]
EvalErrorBuilder<T> error(const Args & ... args) {
return EvalErrorBuilder<T>(*this, args...);
}
private: