Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions src/libexpr/eval-inline.hh
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#include "print.hh"
#include "eval.hh"
#include "eval-error.hh"
#include "eval-settings.hh"

namespace nix {

Expand Down Expand Up @@ -138,5 +139,12 @@ inline void EvalState::forceList(Value & v, const PosIdx pos, std::string_view e
}
}

[[gnu::always_inline]]
inline CallDepth EvalState::addCallDepth(const PosIdx pos) {
if (callDepth > settings.maxCallDepth)
error<EvalError>("stack overflow; max-call-depth exceeded").atPos(pos).debugThrow();

return CallDepth(callDepth);
};

}
38 changes: 20 additions & 18 deletions src/libexpr/eval.cc
Original file line number Diff line number Diff line change
Expand Up @@ -616,6 +616,25 @@ std::optional<EvalState::Doc> EvalState::getDoc(Value & v)
strdup(ss.data()),
};
}
if (isFunctor(v)) {
try {
Value & functor = *v.attrs()->find(sFunctor)->value;
Value * vp = &v;
Value partiallyApplied;
// The first paramater is not user-provided, and may be
// handled by code that is opaque to the user, like lib.const = x: y: y;
// So preferably we show docs that are relevant to the
// "partially applied" function returned by e.g. `const`.
// We apply the first argument:
callFunction(functor, 1, &vp, partiallyApplied, noPos);
auto _level = addCallDepth(noPos);
return getDoc(partiallyApplied);
}
catch (Error & e) {
e.addTrace(nullptr, "while partially calling '%1%' to retrieve documentation", "__functor");
throw;
}
}
return {};
}

Expand Down Expand Up @@ -1471,26 +1490,9 @@ void ExprLambda::eval(EvalState & state, Env & env, Value & v)
v.mkLambda(&env, this);
}

namespace {
/** Increments a count on construction and decrements on destruction.
*/
class CallDepth {
size_t & count;
public:
CallDepth(size_t & count) : count(count) {
++count;
}
~CallDepth() {
--count;
}
};
};

void EvalState::callFunction(Value & fun, size_t nrArgs, Value * * args, Value & vRes, const PosIdx pos)
{
if (callDepth > settings.maxCallDepth)
error<EvalError>("stack overflow; max-call-depth exceeded").atPos(pos).debugThrow();
CallDepth _level(callDepth);
auto _level = addCallDepth(pos);

auto trace = settings.traceFunctionCalls
? std::make_unique<FunctionCallTrace>(positions[pos])
Expand Down
26 changes: 26 additions & 0 deletions src/libexpr/eval.hh
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,21 @@ namespace eval_cache {
class EvalCache;
}

/**
* Increments a count on construction and decrements on destruction.
*/
class CallDepth {
size_t & count;

public:
CallDepth(size_t & count) : count(count) {
++count;
}
~CallDepth() {
--count;
}
};

/**
* Function that implements a primop.
*/
Expand Down Expand Up @@ -625,6 +640,12 @@ public:
const char * doc;
};

/**
* Retrieve the documentation for a value. This will evaluate the value if
* it is a thunk, and it will partially apply __functor if applicable.
*
* @param v The value to get the documentation for.
*/
std::optional<Doc> getDoc(Value & v);

private:
Expand All @@ -649,6 +670,11 @@ private:

public:

/**
* Check that the call depth is within limits, and increment it, until the returned object is destroyed.
*/
inline CallDepth addCallDepth(const PosIdx pos);

/**
* Do a deep equality test between two values. That is, list
* elements and attributes are compared recursively.
Expand Down
101 changes: 101 additions & 0 deletions tests/functional/repl/doc-functor.expected
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
Nix <nix version>
Type :? for help.

nix-repl> :l doc-functor.nix
Added <number omitted> variables.

nix-repl> :doc multiplier
Function `__functor`\
… defined at /path/to/tests/functional/repl/doc-functor.nix:12:23


Multiply the argument by the factor stored in the factor attribute.

nix-repl> :doc doubler
Function `multiply`\
… defined at /path/to/tests/functional/repl/doc-functor.nix:5:17


Look, it's just like a function!

nix-repl> :doc recursive
Function `__functor`\
… defined at /path/to/tests/functional/repl/doc-functor.nix:77:23


This looks bad, but the docs are ok because of the eta expansion.

nix-repl> :doc recursive2
error:
… while partially calling '__functor' to retrieve documentation

… while calling '__functor'
at /path/to/tests/functional/repl/doc-functor.nix:85:17:
84| */
85| __functor = self: self.__functor self;
| ^
86| };

… from call site
at /path/to/tests/functional/repl/doc-functor.nix:85:23:
84| */
85| __functor = self: self.__functor self;
| ^
86| };

(19999 duplicate frames omitted)

error: stack overflow; max-call-depth exceeded
at /path/to/tests/functional/repl/doc-functor.nix:85:23:
84| */
85| __functor = self: self.__functor self;
| ^
86| };

nix-repl> :doc diverging
error:
… while partially calling '__functor' to retrieve documentation

(10000 duplicate frames omitted)

… while calling '__functor'
at /path/to/tests/functional/repl/doc-functor.nix:97:19:
96| f = x: {
97| __functor = self: (f (x + 1));
| ^
98| };

error: stack overflow; max-call-depth exceeded
at /path/to/tests/functional/repl/doc-functor.nix:97:26:
96| f = x: {
97| __functor = self: (f (x + 1));
| ^
98| };

nix-repl> :doc helper
Function `square`\
… defined at /path/to/tests/functional/repl/doc-functor.nix:36:12


Compute x^2

nix-repl> :doc helper2
Function `__functor`\
… defined at /path/to/tests/functional/repl/doc-functor.nix:45:23


This is a function that can be overridden.

nix-repl> :doc lib.helper3
Function `__functor`\
… defined at /path/to/tests/functional/repl/doc-functor.nix:45:23


This is a function that can be overridden.

nix-repl> :doc helper3
Function `__functor`\
… defined at /path/to/tests/functional/repl/doc-functor.nix:45:23


This is a function that can be overridden.
10 changes: 10 additions & 0 deletions tests/functional/repl/doc-functor.in
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
:l doc-functor.nix
:doc multiplier
:doc doubler
:doc recursive
:doc recursive2
:doc diverging
:doc helper
:doc helper2
:doc lib.helper3
:doc helper3
101 changes: 101 additions & 0 deletions tests/functional/repl/doc-functor.nix
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
rec {
/**
Look, it's just like a function!
*/
multiply = p: q: p * q;

multiplier = {
factor = 2;
/**
Multiply the argument by the factor stored in the factor attribute.
*/
__functor = self: x: x * self.factor;
};

doubler = {
description = "bla";
/**
Multiply by two. This doc probably won't be rendered because the
returned partial application won't have any reference to this location;
only pointing to the second lambda in the multiply function.
*/
__functor = self: multiply 2;
};

makeOverridable = f: {
/**
This is a function that can be overridden.
*/
__functor = self: f;
override = throw "not implemented";
};

/**
Compute x^2
*/
square = x: x * x;

helper = makeOverridable square;

# Somewhat analogous to the Nixpkgs makeOverridable function.
makeVeryOverridable = f: {
/**
This is a function that can be overridden.
*/
__functor = self: arg: f arg // { override = throw "not implemented"; overrideAttrs = throw "not implemented"; };
override = throw "not implemented";
};

helper2 = makeVeryOverridable square;

# The RFC might be ambiguous here. The doc comment from makeVeryOverridable
# is "inner" in terms of values, but not inner in terms of expressions.
# Returning the following attribute comment might be allowed.
# TODO: I suppose we could look whether the attribute value expression
# contains a doc, and if not, return the attribute comment anyway?
Comment on lines +51 to +55
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hsjobeki What do you think?
Currently we follow the disambiguation rule at the value level.
Was that the intent, or should we only apply it at the expression level, so that in cases like this we can resolve to a more specific comment.
Or perhaps more simply, when the :doc query is specifically for an attribute, perhaps we should just prefer the attribute-based lookup?
I'm also considering the option that we should just show everything and treat the rule as a preference or priority.

nix-repl> :doc lib.helper3
Function `__functor`\
  … defined at /path/to/tests/functional/repl/doc-functor.nix:45:23


This is a function that can be overridden.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or perhaps more simply, when the :doc query is specifically for an attribute, perhaps we should just prefer the attribute-based lookup?

Yes. But functors are polymorph. I would show (maybe concatenate) both, or add an option for the user to decide what he is interested in.

As a user i would expect :doc lib.helper3 to yield.

:doc lib.helper3
->  Compute x^3

The property that the function is wrapped with makeVeryOverridable is not very interesting.
But i dont know how to solve this issue yet.

Copy link
Contributor

@hsjobeki hsjobeki Aug 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For example this one edge case that i couldn't solve on a value level, because lamdbas are opaque.

If you try to unwrap this at the value level:
pkgs.fetchFromGitHub
-> (position)
https://github.com/nixos/nixpkgs/blob/master/lib/customisation.nix#L134:C17

Maybe we need some annotation for higher order functions like this. (like @wrapper, so we know its just wrapping some inner function that we are interested in)
Or we figure out some way for additional tracking on the expression level.


/**
Compute x^3
*/
lib.helper3 = makeVeryOverridable (x: x * x * x);

/**
Compute x^3...
*/
helper3 = makeVeryOverridable (x: x * x * x);


# ------

# getDoc traverses a potentially infinite structure in case of __functor, so
# we need to test with recursive inputs and diverging inputs.

recursive = {
/**
This looks bad, but the docs are ok because of the eta expansion.
*/
__functor = self: x: self x;
};

recursive2 = {
/**
Docs probably won't work in this case, because the "partial" application
of self results in an infinite recursion.
*/
__functor = self: self.__functor self;
};

diverging = let
/**
Docs probably won't work in this case, because the "partial" application
of self results in an diverging computation that causes a stack overflow.
It's not an infinite recursion because each call is different.
This must be handled by the documentation retrieval logic, as it
reimplements the __functor invocation to be partial.
*/
f = x: {
__functor = self: (f (x + 1));
};
in f null;

}