Let's hope this doesn't causes build failures for e.g. musl: I just
know it's good on macOS and our Linux CI.
It's been a long time.
One fix this brings, is I discovered we #include assert.h or cassert
in a lot of places. If those ever happen to be in a file that doesn't
include common.h, or we are before common.h gets included, we're
unawaringly working with the system 'assert' macro again, which
may get disabled for debug builds or at least has different
behavior on crash. We undef 'assert' and redefine it in common.h.
Those were all eliminated, except in one catch-22 spot for
maybe.h: it can't include common.h. A fix might be to
make a fish_assert.h that *usually* common.h exports.
ESCAPE_ALL is not really a helpful name. Also it's the most common flag.
Let's make it the default so we can remove this unhelpful name.
While at it, let's add a default value for the flags argument, which helps
most callers.
The absence of ESCAPE_ALL makes it only escape nonprintable characters
(with some exceptions). We use this for displaying strings in the completion
pager as well as for the human-readable output of "set", "set -S", "bind"
and "functions".
No functional change.
This concerns what happens if the user types e.g. `grep --i` and grep or
its completions have not yet been loaded. Previously we would "bounce to
the main thread" from within the autosuggestion thread to load grep's
completions. However under concurrent execution, this may deadlock as the
main thread is waiting for something else.
In the new implementation, complete simply records the commands that it
would autoload, and returns them back to the caller, where the caller can
decide how to handle them.
In general iothread_perform_on_main risks deadlock under concurrent
execution and we should try to get rid of it.
There should be no user-visible change from this fix.
This used to call exec_subshell, which has two issues:
1. It creates a command substitution block which shows up in a stack
trace
2. It does much more work than necessary
This removes a useless "in command substitution" from an error message
in an autoloaded file, and it speeds up autoloading a bit (not
measurable in actual benchmarks, but microbenchmarks are 2x).
The ternary expression was causing the list of paths (e.g.
$fish_function_path) to be copied. Avoid that copy with an if statement.
This reduces the time spent in try_autoload from 2.4 sec to 961ms on
the seq_echo benchmark run 1024 times, about 5% improvement.
Oh, C++...
This cleans up how functions are stored and autoloaded. It eliminates the
recursive lock. Instead there is a single normal owning_lock that protects
the entirety of the function data. Autoloading is re-implemented via the
new autoloader_t.
autoloader_t will be the reimplementation of autoloading. Crucically it no
longer manages any locking or loading itself; instead all locking and loading
is performed by clients. This makes it easier to test and helps limit its
responsibilities.
autoloading has a "feature" where functions are removed in an LRU-fashion.
But there's hardly any benefit in removing autoloaded functions. Just stop
doing it.
fish's signal handlers are now sufficiently innocuous that there should
be no reason to block signals (outside of temporarily, when creating a
thread and we need to manipulate the signal mask).
Prior to this fix, autoloads like function and completion autoloads
would check their path variable (like fish_function_path) on every
autoload request. Switch to invalidating it in response to the variable
changing.
This improves time on a microbenchmark:
for i in (seq 50000)
setenv test_env val$i
end
from ~11 seconds to ~6.5 seconds.
Add a fish-specific wrapper around std::mutex that records whether it is
locked in a bool. This is to make ASSERT_IS_LOCKED() simpler (it can just
check the boolean instead of relying on try_lock) which will make Coverity
Scan happier.
Some details: Coverity Scan was complaining about an apparent double-unlock
because it's unaware of the semantics of try_lock(). Specifically fish
asserts that a lock is locked by asserting that try_lock fails; if it
succeeds fish prints an error and then unlocks the lock (so as not to leave
it locked). This unlock is of course correct, but it confused Coverity Scan.
This eliminates the "missing" notion of env_var_t. Instead
env_get returns a maybe_t<env_var_t>, which forces callers to
handle the possibility that the variable is missing.
commit 50f414a45d58fcab664ff662dd27befcfa0fdd95
Author: Mahmoud Al-Qudsi <mqudsi@neosmart.net>
Date: Sat Aug 19 13:43:35 2017 -0500
Converted file_id_t set to unordered_set with custom hash
commit 83ef2dd7cc1bc3e4fdf0b2d3546d6811326cc3c9
Author: Mahmoud Al-Qudsi <mqudsi@neosmart.net>
Date: Sat Aug 19 13:43:14 2017 -0500
Converted remaining set<wcstring> to unordered_set<wcstring>
commit 053da88f933f27505b3cf4810402e2a2be070203
Author: Mahmoud Al-Qudsi <mqudsi@neosmart.net>
Date: Sat Aug 19 13:29:21 2017 -0500
Switched function sets to unordered_set
commit d469742a14ac99599022a9258cda8255178826b5
Author: Mahmoud Al-Qudsi <mqudsi@neosmart.net>
Date: Sat Aug 19 13:21:32 2017 -0500
Converted list of modified variables to an unordered set
commit 5c06f866beeafb23878b1a932c7cd2558412c283
Author: Mahmoud Al-Qudsi <mqudsi@neosmart.net>
Date: Sat Aug 19 13:15:20 2017 -0500
Convert const_string_set_t to std::unordered_set
As it is a readonly-list of raw character pointer strings (not
wcstring), this necessitated the addition of a hashing function since
the C++ standard library does not come with a char pointer hash
function.
To that end, a zlib-licensed [0] port of the excellent, lightweight
XXHash family of 32- and 64-bit hashing algorithms in the form of a C++
header-only include library has been included. XXHash32/64 is pretty
much universally the fastest hashing library for general purpose
applications, and has been thoroughly vetted and is used in countless
open source projects. The single-header version of this library makes it
a lot simpler to include in the fish project, and the license
compatibility with fish' GPLv2 and the zero-lib nature should make it an
easy decision.
std::unordered_set brings a massive speedup as compared to the default
std::set, and the further use of the fast XXHash library to provide the
string hashing should make all forms of string lookups in fish
significantly faster (to a user-noticeable extent).
0: http://create.stephan-brumme.com/about.html
commit 30d7710be8f0c23a4d42f7e713fcb7850f99036e
Author: Mahmoud Al-Qudsi <mqudsi@neosmart.net>
Date: Sat Aug 19 12:29:39 2017 -0500
Using std::unordered_set for completions backing store
While the completions shown to the user are sorted, their storage in
memory does not need to be since they are re-sorted before they are
shown in completions.cpp.
commit 695e83331d7a60ba188e57f6ea0d9b6da54860c6
Author: Mahmoud Al-Qudsi <mqudsi@neosmart.net>
Date: Sat Aug 19 12:06:53 2017 -0500
Updated is_loading to use unordered_set
No longer using RAII wrappers around pthread_mutex_t and pthread_cond_t
in favor of the C++11 std::mutex, std::recursive_mutex, and
std::condition_variable data types.
This is the first step to implementing issue #4200 is to stop subclassing
env_var_t from wcstring. Not too surprisingly doing this identified
several places that were incorrectly treating env_var_t and wcstring as
interchangeable types. I'm not talking about those places that passed
an env_var_t instance to a function that takes a wcstring. I'm talking
about doing things like assigning the former to the latter type, relying
on the implicit conversion, and thus losing information.
We also rename `env_get_string()` to `env_get()` for symmetry with
`env_set()` and to make it clear the function does not return a string.
I recently upgraded the software on my macOS server and was dismayed to
see that cppcheck reported a huge number of format string errors due to
mismatches between the format string and its arguments from calls to
`assert()`. It turns out they are due to the macOS header using `%lu`
for the line number which is obviously wrong since it is using the C
preprocessor `__LINE__` symbol which evaluates to a signed int.
I also noticed that the macOS implementation writes to stdout, rather
than stderr. It also uses `printf()` which can be a problem on some
platforms if the stream is already in wide mode which is the normal case
for fish.
So implement our own `assert()` implementation. This also eliminates
double-negative warnings that we get from some of our calls to
`assert()` on some platforms by oclint.
Also reimplement the `DIE()` macro in terms of our internal
implementation.
Rewrite `assert(0 && msg)` statements to `DIE(msg)` for clarity and to
eliminate oclint warnings about constant expressions.
Fixes#3276, albeit not in the fashion I originally envisioned.
I'm starting to wonder if IWYU is worth the effort. Nonetheless, this
makes it lint clean on macOS and reduces the number of warnings on
FreeBSD and Linux.
This was an old experiment to compile scripts directly into the
shell itself, reducing the amount of I/O performed at startup.
It has not been used for a long time. Time to remove it.
We should never use stdio functions that use stdout implicitly. Saving a
few characters isn't worth the inconsistency. Too, using the forms such
as `fwprintf()` which take an explicit stream makes it easier to find
the places we write to stdout versus stderr.
Fixes#3728
