Cleap up owning_lock interface

Use some operator overloading to avoid the weird interface of acquired_lock.

src/builtin_commandline.cpp

@@ -55,30 +55,27 @@ static owning_lock<wcstring_list_t> &get_transient_stack() {
 }
 
 static bool get_top_transient(wcstring *out_result) {
-    auto &&locked = get_transient_stack().acquire();
-    wcstring_list_t &stack = locked.value;
-    if (stack.empty()) {
+    auto stack = get_transient_stack().acquire();
+    if (stack->empty()) {
         return false;
     }
-    out_result->assign(stack.back());
+    out_result->assign(stack->back());
     return true;
 }
 
 builtin_commandline_scoped_transient_t::builtin_commandline_scoped_transient_t(
     const wcstring &cmd) {
     ASSERT_IS_MAIN_THREAD();
-    auto &&locked = get_transient_stack().acquire();
-    wcstring_list_t &stack = locked.value;
-    stack.push_back(cmd);
-    this->token = stack.size();
+    auto stack = get_transient_stack().acquire();
+    stack->push_back(cmd);
+    this->token = stack->size();
 }
 
 builtin_commandline_scoped_transient_t::~builtin_commandline_scoped_transient_t() {
     ASSERT_IS_MAIN_THREAD();
-    auto &&locked = get_transient_stack().acquire();
-    wcstring_list_t &stack = locked.value;
-    assert(this->token == stack.size());
-    stack.pop_back();
+    auto stack = get_transient_stack().acquire();
+    assert(this->token == stack->size());
+    stack->pop_back();
 }
 
 /// Replace/append/insert the selection with/at/after the specified string.

src/common.h

@@ -598,20 +598,25 @@ typedef std::lock_guard<std::recursive_mutex> scoped_rlock;
 //
 template <typename DATA>
 class acquired_lock {
-    scoped_lock lock;
-    acquired_lock(fish_mutex_t &lk, DATA *v) : lock(lk), value(*v) {}
+    std::unique_lock<fish_mutex_t> lock;
+    acquired_lock(fish_mutex_t &lk, DATA *v) : lock(lk), value(v) {}
 
     template <typename T>
     friend class owning_lock;
 
+    DATA *value;
+
    public:
-    // No copying, move only
+    // No copying, move construction only
     acquired_lock &operator=(const acquired_lock &) = delete;
     acquired_lock(const acquired_lock &) = delete;
     acquired_lock(acquired_lock &&) = default;
     acquired_lock &operator=(acquired_lock &&) = default;
 
-    DATA &value;
+    DATA *operator->() { return value; }
+    const DATA *operator->() const { return value; }
+    DATA &operator*() { return *value; }
+    const DATA &operator*() const { return *value; }
 };
 
 // A lock that owns a piece of data

src/history.cpp

@@ -816,8 +816,8 @@ history_t &history_collection_t::get_creating(const wcstring &name) {
     // Return a history for the given name, creating it if necessary
     // Note that histories are currently never deleted, so we can return a reference to them without
     // using something like shared_ptr
-    auto &&hs = histories.acquire();
-    std::unique_ptr<history_t> &hist = hs.value[name];
+    auto hs = histories.acquire();
+    std::unique_ptr<history_t> &hist = (*hs)[name];
     if (!hist) {
         hist = make_unique<history_t>(name);
     }
@@ -1849,8 +1849,8 @@ void history_t::incorporate_external_changes() {
 
 void history_collection_t::save() {
     // Save all histories
-    auto &&h = histories.acquire();
-    for (auto &p : h.value) {
+    auto hists = histories.acquire();
+    for (auto &p : *hists) {
         p.second->save();
     }
 }

src/intern.cpp

@@ -21,17 +21,15 @@ static const wchar_t *intern_with_dup(const wchar_t *in, bool dup) {
     if (!in) return NULL;
 
     debug(5, L"intern %ls", in);
-    auto &&lock_string_table = string_table.acquire();
-    std::vector<const wchar_t *> &string_table = lock_string_table.value;
+    auto table = string_table.acquire();
 
     const wchar_t *result;
-    auto iter =
-        std::lower_bound(string_table.begin(), string_table.end(), in, string_less_than_string);
-    if (iter != string_table.end() && wcscmp(*iter, in) == 0) {
+    auto iter = std::lower_bound(table->begin(), table->end(), in, string_less_than_string);
+    if (iter != table->end() && wcscmp(*iter, in) == 0) {
         result = *iter;
     } else {
         result = dup ? wcsdup(in) : in;
-        string_table.insert(iter, result);
+        table->insert(iter, result);
     }
     return result;
 }

src/iothread.cpp

@@ -97,18 +97,17 @@ static void iothread_init() {
 }
 
 static bool dequeue_spawn_request(spawn_request_t *result) {
-    auto &&locker = s_spawn_requests.acquire();
-    thread_data_t &td = locker.value;
-    if (!td.request_queue.empty()) {
-        *result = std::move(td.request_queue.front());
-        td.request_queue.pop();
+    auto requests = s_spawn_requests.acquire();
+    if (!requests->request_queue.empty()) {
+        *result = std::move(requests->request_queue.front());
+        requests->request_queue.pop();
         return true;
     }
     return false;
 }
 
 static void enqueue_thread_result(spawn_request_t req) {
-    s_result_queue.acquire().value.push(std::move(req));
+    s_result_queue.acquire()->push(std::move(req));
 }
 
 static void *this_thread() { return (void *)(intptr_t)pthread_self(); }
@@ -140,7 +139,7 @@ static void *iothread_worker(void *unused) {
     // committed to not handling anything else. Therefore, we have to decrement
     // the thread count under the lock, which we still hold. Likewise, the main thread must
     // check the value under the lock.
-    int new_thread_count = --s_spawn_requests.acquire().value.thread_count;
+    int new_thread_count = --s_spawn_requests.acquire()->thread_count;
     assert(new_thread_count >= 0);
 
     debug(5, "pthread %p exiting", this_thread());
@@ -180,14 +179,13 @@ int iothread_perform_impl(void_function_t &&func, void_function_t &&completion)
     bool spawn_new_thread = false;
     {
         // Lock around a local region.
-        auto &&locker = s_spawn_requests.acquire();
-        thread_data_t &td = locker.value;
-        td.request_queue.push(std::move(req));
-        if (td.thread_count < IO_MAX_THREADS) {
-            td.thread_count++;
+        auto spawn_reqs = s_spawn_requests.acquire();
+        spawn_reqs->request_queue.push(std::move(req));
+        if (spawn_reqs->thread_count < IO_MAX_THREADS) {
+            spawn_reqs->thread_count++;
             spawn_new_thread = true;
         }
-        local_thread_count = td.thread_count;
+        local_thread_count = spawn_reqs->thread_count;
     }
 
     // Kick off the thread if we decided to do so.
@@ -249,7 +247,7 @@ void iothread_drain_all() {
 #endif
 
     // Nasty polling via select().
-    while (s_spawn_requests.acquire().value.thread_count > 0) {
+    while (s_spawn_requests.acquire()->thread_count > 0) {
         if (iothread_wait_for_pending_completions(1000)) {
             iothread_service_completion();
         }
@@ -300,7 +298,7 @@ static void iothread_service_main_thread_requests() {
 static void iothread_service_result_queue() {
     // Move the queue to a local variable.
     std::queue<spawn_request_t> result_queue;
-    s_result_queue.acquire().value.swap(result_queue);
+    (*s_result_queue.acquire()).swap(result_queue);
 
     // Perform each completion in order
     while (!result_queue.empty()) {

src/proc.cpp

@@ -163,39 +163,37 @@ int proc_get_last_status() { return last_status; }
 static owning_lock<std::vector<bool>> locked_consumed_job_ids;
 
 job_id_t acquire_job_id() {
-    auto &&locker = locked_consumed_job_ids.acquire();
-    std::vector<bool> &consumed_job_ids = locker.value;
+    auto consumed_job_ids = locked_consumed_job_ids.acquire();
 
     // Find the index of the first 0 slot.
-    auto slot = std::find(consumed_job_ids.begin(), consumed_job_ids.end(), false);
-    if (slot != consumed_job_ids.end()) {
+    auto slot = std::find(consumed_job_ids->begin(), consumed_job_ids->end(), false);
+    if (slot != consumed_job_ids->end()) {
         // We found a slot. Note that slot 0 corresponds to job ID 1.
         *slot = true;
-        return (job_id_t)(slot - consumed_job_ids.begin() + 1);
+        return (job_id_t)(slot - consumed_job_ids->begin() + 1);
     }
 
     // We did not find a slot; create a new slot. The size of the vector is now the job ID
     // (since it is one larger than the slot).
-    consumed_job_ids.push_back(true);
-    return (job_id_t)consumed_job_ids.size();
+    consumed_job_ids->push_back(true);
+    return (job_id_t)consumed_job_ids->size();
 }
 
 void release_job_id(job_id_t jid) {
     assert(jid > 0);
-    auto &&locker = locked_consumed_job_ids.acquire();
-    std::vector<bool> &consumed_job_ids = locker.value;
-    size_t slot = (size_t)(jid - 1), count = consumed_job_ids.size();
+    auto consumed_job_ids = locked_consumed_job_ids.acquire();
+    size_t slot = (size_t)(jid - 1), count = consumed_job_ids->size();
 
     // Make sure this slot is within our vector and is currently set to consumed.
     assert(slot < count);
-    assert(consumed_job_ids.at(slot) == true);
+    assert(consumed_job_ids->at(slot) == true);
 
     // Clear it and then resize the vector to eliminate unused trailing job IDs.
-    consumed_job_ids.at(slot) = false;
+    consumed_job_ids->at(slot) = false;
     while (count--) {
-        if (consumed_job_ids.at(count)) break;
+        if (consumed_job_ids->at(count)) break;
     }
-    consumed_job_ids.resize(count + 1);
+    consumed_job_ids->resize(count + 1);
 }
 
 job_t *job_get(job_id_t id) {

src/wutil.cpp

@@ -448,8 +448,8 @@ const wcstring &wgettext(const wchar_t *in) {
     wcstring key = in;
 
     wgettext_init_if_necessary();
-    auto &&wmap = wgettext_map.acquire();
-    wcstring &val = wmap.value[key];
+    auto wmap = wgettext_map.acquire();
+    wcstring &val = (*wmap)[key];
     if (val.empty()) {
         cstring mbs_in = wcs2string(key);
         char *out = fish_gettext(mbs_in.c_str());