Correctly export empty universal variables

Fixes #5992

CHANGELOG.md

@@ -18,6 +18,7 @@
 - `string` has a new `collect` subcommand that disables newline-splitting on its input. This is meant to be used as the end of a command substitution pipeline to produce a single output argument potentially containing internal newlines, such as `set output (some-cmd | string collect)`. Any trailing newlines are trimmed, just like `"$(cmd)"` substitution in sh. It also supports a `--no-trim-newlines` flag to disable trailing newline trimming, which may be useful when doing something like `set contents (cat filename | string collect -N)` (#159).
 - More of the documentation, including the tutorial, is now available as man pages as well.
 - Local values for `fish_complete_path` and `fish_function_path` are now ignored; only their global values are respected.
+- Empty universal variables may now be exported (#5992).
 
 ### Syntax changes and new commands
 - Brace expansion now only takes place if the braces include a "," or a variable expansion, so things like `git reset HEAD@{0}` now work (#5869).

src/env.cpp

@@ -638,12 +638,10 @@ std::shared_ptr<const null_terminated_array_t<char>> env_scoped_impl_t::create_e
         const wcstring_list_t uni = uvars()->get_names(true, false);
         for (const wcstring &key : uni) {
             auto var = uvars()->get(key);
-
+            assert(var && "Variable should be present in uvars");
-            if (!var.missing_or_empty()) {
+            // Note that std::map::insert does NOT overwrite a value already in the map,
-                // Note that std::map::insert does NOT overwrite a value already in the map,
+            // which we depend on here.
-                // which we depend on here.
+            vals.insert(std::make_pair(key, *var));
-                vals.insert(std::pair<wcstring, env_var_t>(key, *var));
-            }
         }
     }
 

src/env_universal_common.cpp

@@ -300,10 +300,9 @@ bool env_universal_t::remove(const wcstring &key) {
 wcstring_list_t env_universal_t::get_names(bool show_exported, bool show_unexported) const {
     wcstring_list_t result;
     scoped_lock locker(lock);
-    var_table_t::const_iterator iter;
+    for (const auto &kv : vars) {
-    for (iter = vars.begin(); iter != vars.end(); ++iter) {
+        const wcstring &key = kv.first;
-        const wcstring &key = iter->first;
+        const env_var_t &var = kv.second;
-        const env_var_t &var = iter->second;
         if ((var.exports() && show_exported) || (!var.exports() && show_unexported)) {
             result.push_back(key);
         }

tests/checks/set.fish

@@ -405,6 +405,16 @@ echo "$__fish_test_path2" $__fish_test_path2
 
 set -e __fish_test_path2
 
+# Test empty uvars (#5992)
+set -Ux __fish_empty_uvar
+set -Uq __fish_empty_uvar
+echo $status
+# CHECK: 0
+$FISH -c 'set -Uq __fish_empty_uvar; echo $status'
+# CHECK: 0
+env | grep __fish_empty_uvar
+# CHECK: __fish_empty_uvar=
+
 # Variable names in other commands
 # Test invalid variable names in loops (#5800)
 for a,b in y 1 z 3