From b3599166ad98c3cad304183bd7cc7bc280522e71 Mon Sep 17 00:00:00 2001 From: eldritch horrors Date: Mon, 18 Mar 2024 20:03:31 +0100 Subject: [PATCH] libexpr: sort binding name in debugger not doing this exposes the binding name order to the annoying interference of parse order on symbol order, which wouldn't be so bad if it didn't make the tests less reliable and, importantly, dependent on linker behavior (due to primop initialization being done in static initializer, and the order of static initializers being defined only within a single translation unit). fixes #143 Change-Id: I3cf417893fbcf19e9ad3ff8986deb7cbcf3ca511 --- src/libexpr/eval.cc | 30 ++++++++++++------- .../data/regression_9918.test | 4 +-- .../data/stack_vars.test | 8 ++--- 3 files changed, 26 insertions(+), 16 deletions(-) diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 7cf33aec4..bef0effb6 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -677,12 +677,21 @@ std::optional EvalState::getDoc(Value & v) } +static std::set sortedBindingNames(const SymbolTable & st, const StaticEnv & se) +{ + std::set bindings; + for (auto [symbol, displ] : se.vars) + bindings.emplace(st[symbol]); + return bindings; +} + + // just for the current level of StaticEnv, not the whole chain. void printStaticEnvBindings(const SymbolTable & st, const StaticEnv & se) { std::cout << ANSI_MAGENTA; - for (auto & i : se.vars) - std::cout << st[i.first] << " "; + for (auto & i : sortedBindingNames(st, se)) + std::cout << i << " "; std::cout << ANSI_NORMAL; std::cout << std::endl; } @@ -691,13 +700,14 @@ void printStaticEnvBindings(const SymbolTable & st, const StaticEnv & se) void printWithBindings(const SymbolTable & st, const Env & env) { if (!env.values[0]->isThunk()) { + std::set bindings; + for (const auto & attr : *env.values[0]->attrs) + bindings.emplace(st[attr.name]); + std::cout << "with: "; std::cout << ANSI_MAGENTA; - Bindings::iterator j = env.values[0]->attrs->begin(); - while (j != env.values[0]->attrs->end()) { - std::cout << st[j->name] << " "; - ++j; - } + for (auto & i : bindings) + std::cout << i << " "; std::cout << ANSI_NORMAL; std::cout << std::endl; } @@ -718,9 +728,9 @@ void printEnvBindings(const SymbolTable & st, const StaticEnv & se, const Env & std::cout << ANSI_MAGENTA; // for the top level, don't print the double underscore ones; // they are in builtins. - for (auto & i : se.vars) - if (!std::string_view(st[i.first]).starts_with("__")) - std::cout << st[i.first] << " "; + for (auto & i : sortedBindingNames(st, se)) + if (!i.starts_with("__")) + std::cout << i << " "; std::cout << ANSI_NORMAL; std::cout << std::endl; if (se.isWith) diff --git a/tests/functional/repl_characterization/data/regression_9918.test b/tests/functional/repl_characterization/data/regression_9918.test index 4a1931122..c30c405b6 100644 --- a/tests/functional/repl_characterization/data/regression_9918.test +++ b/tests/functional/repl_characterization/data/regression_9918.test @@ -10,7 +10,7 @@ We expect to be able to see locals like r in the debugger: nix-repl> :env Env level 0 - static: x r + static: r x Env level 1 - builtins true false null scopedImport import isNull break abort throw derivationStrict placeholder baseNameOf dirOf removeAttrs map toString fetchMercurial fetchTree fetchTarball fetchGit fromTOML derivation + abort baseNameOf break builtins derivation derivationStrict dirOf false fetchGit fetchMercurial fetchTarball fetchTree fromTOML import isNull map null placeholder removeAttrs scopedImport throw toString true diff --git a/tests/functional/repl_characterization/data/stack_vars.test b/tests/functional/repl_characterization/data/stack_vars.test index 0537f9c03..96ea5fe25 100644 --- a/tests/functional/repl_characterization/data/stack_vars.test +++ b/tests/functional/repl_characterization/data/stack_vars.test @@ -14,7 +14,7 @@ as it is in scope. static: a b Env level 2 - builtins true false null scopedImport import isNull break abort throw derivationStrict placeholder baseNameOf dirOf removeAttrs map toString fetchMercurial fetchTree fetchTarball fetchGit fromTOML derivation + abort baseNameOf break builtins derivation derivationStrict dirOf false fetchGit fetchMercurial fetchTarball fetchTree fromTOML import isNull map null placeholder removeAttrs scopedImport throw toString true nix-repl> meow 2 @@ -35,7 +35,7 @@ If we :st past the frame in the backtrace with the meow in it, the meow should n static: a b Env level 1 - builtins true false null scopedImport import isNull break abort throw derivationStrict placeholder baseNameOf dirOf removeAttrs map toString fetchMercurial fetchTree fetchTarball fetchGit fromTOML derivation + abort baseNameOf break builtins derivation derivationStrict dirOf false fetchGit fetchMercurial fetchTarball fetchTree fromTOML import isNull map null placeholder removeAttrs scopedImport throw toString true nix-repl> :c trace: before inner break @@ -52,7 +52,7 @@ If we :st past the frame in the backtrace with the meow in it, the meow should n static: a b Env level 2 - builtins true false null scopedImport import isNull break abort throw derivationStrict placeholder baseNameOf dirOf removeAttrs map toString fetchMercurial fetchTree fetchTarball fetchGit fromTOML derivation + abort baseNameOf break builtins derivation derivationStrict dirOf false fetchGit fetchMercurial fetchTarball fetchTree fromTOML import isNull map null placeholder removeAttrs scopedImport throw toString true nix-repl> meow' 3 @@ -71,4 +71,4 @@ If we :st past the frame in the backtrace with the meow in it, the meow should n static: a b Env level 1 - builtins true false null scopedImport import isNull break abort throw derivationStrict placeholder baseNameOf dirOf removeAttrs map toString fetchMercurial fetchTree fetchTarball fetchGit fromTOML derivation + abort baseNameOf break builtins derivation derivationStrict dirOf false fetchGit fetchMercurial fetchTarball fetchTree fromTOML import isNull map null placeholder removeAttrs scopedImport throw toString true