This doesn't work.
The real thing that tells if something is read-only is
electric_var_t::readonly().
This wasn't used, and we provide no way to make a variable read-only,
which makes this an unnecessary footgun.
Several functions including wgetopt and execve operate on null-terminated
arrays of nul-terminated pointers: a list of pointers to C strings where
the last pointer is null. Prior to this change, each process_t stored its
argv in such an array. This had two problems:
1. It was awkward to work with this type, instead of using std::vector,
etc.
2. The process's arguments would be rearranged by builtins which is
surprising
Our null terminated arrays were built around a fancy type that would copy
input strings and also generate an array of pointers to them, in one big
allocation.
Switch to a new model where we construct an array of pointers over
existing strings. So you can supply a `vector<string>` and now
`null_terminated_array_t` will just make a list of pointers to them. Now
processes can just store their argv in a familiar wcstring_list_t.
Profiling revealed string comparison in variable lookups to be a
significant hotspot. This change causes `make test` to complete ~4.5%
faster per `hyperfine`.
fish will react to certain variable modifications, such as "TZ." Only do
this if the main stack is modified. This has no effect now because there
is always a single stack, but will become important when concurrent
execution is supported.
PATH and CDPATH have special behavior around empty elements. Express this
directly in env_stack_t::set rather than via variable dispatch; this is
cleaner.
Prior to this fix, fish would invalidate the exported variable list
whenever an exported variable changes. However we soon will not have a
single "exported variable list." If a global variable changes, it is
infeasible to find all exported variable lists and invalidate them.
Switch to a new model where we store a list of generation counts. Every
time an exported variable changes, the node gets a new generation. If the
current generation list does not match the cached one, then we know that
our exported variable list is stale.
This makes the following changes:
1. Events in background threads are executed in those threads, instead of
being silently dropped
2. Blocked events are now per-parser instead of global
3. Events are posted in builtin_set instead of within the environment stack
The last one means that we no longer support event handlers for implicit
sets like (example) argv. Instead only the `set` builtin (and also `cd`)
post variable-change events.
Events from universal variable changes are still not fully rationalized.
Prior to this fix, a function_block stored a process_t, which was only used
when printing backtraces. Switch this to an array of arguments, and make
various other cleanups around null terminated argument arrays.
This runs build_tools/style.fish, which runs clang-format on C++, fish_indent on fish and (new) black on python.
If anything is wrong with the formatting, we should fix the tools, but automated formatting is worth it.
This was a sort of side channel that was only used to propagate redraws
after universal variable changes. We can eliminate it and handle these
more directly.
env_scoped_t lives between environment_t and env_stack_t.
It represents the read-only logic of env_stack_t and will be used to back
the new environment snapshot implementation.
This new file is supposed to encapsulate all of the logic around
reacting to variable changes, as opposed to the environment core.
This is to help break up the env.cpp monolith.
This switches env_var_t to be an immutable value type, and stores its
contents via a shared_ptr. This eliminates string copying when fetching
env_var_t values.
For some reason, we have two places where a variable can be read-only:
- By key in env.cpp:is_read_only(), which is checked via set*
- By flag on the actual env_var_t, which is checked e.g. in
parse_execution
The latter didn't happen for non-electric variables like hostname,
because they used the default constructor, because they were
constructed via operator[] (or some such C++-iness).
This caused for-loops to crash on an assert if they used a
non-electric read-only var like $hostname or $SHLVL.
Instead, we explicitly set the flag.
We might want to remove one of the two read-only checks, or something?
Fixes#5548.
This requires threading environment_t through many places, such as completions
and history. We introduce null_environment_t for when the environment isn't
important.
