[oil-language] Change some hidden assignments to SetLocalShopt()

And use SetVarShopt() for x=y, etc.

core/process.py

@@ -157,7 +157,7 @@ def _WriteFdToMem(self, fd_name, fd):
     # type: (str, int) -> None
     if self.mem:
       # setvar, not setref
-      state.SetVar(self.mem, lvalue.Named(fd_name), value.Str(str(fd)))
+      state.SetLocalShopt(self.mem, lvalue.Named(fd_name), value.Str(str(fd)))
 
   def _ReadFdFromMem(self, fd_name):
     # type: (str) -> int

core/state.py

@@ -1243,7 +1243,7 @@ def IsAssocArray(self, name):
     We need to know this to evaluate the index expression properly -- should it
     be coerced to an integer or not?
     """
-    cell, _, _ = self._ResolveNameOrRef(name, self._LookupMode())
+    cell, _, _ = self._ResolveNameOrRef(name, self.DynamicOrLocalGlobal())
     if cell:
       if cell.val.tag_() == value_e.AssocArray:  # foo=([key]=value)
         return True
@@ -1516,7 +1516,7 @@ def GetValue(self, name, lookup_mode=scope_e.Shopt):
     assert isinstance(name, str), name
 
     if lookup_mode == scope_e.Shopt:
-      lookup_mode = self._LookupMode()
+      lookup_mode = self.DynamicOrLocalGlobal()
 
     #log('mode %s', lookup_mode)
 
@@ -1622,7 +1622,7 @@ def GetCell(self, name, lookup_mode=scope_e.Shopt):
       - to test of 'TZ' is exported in printf?  Why?
     """
     if lookup_mode == scope_e.Shopt:
-      lookup_mode = self._LookupMode()
+      lookup_mode = self.DynamicOrLocalGlobal()
 
     cell, _ = self._ResolveNameOnly(name, lookup_mode)
     return cell
@@ -1719,21 +1719,28 @@ def Unset(self, lval, strict):
 
     return True
 
-  def _LookupMode(self):
+  def DynamicOrLocalGlobal(self):
     # type: () -> scope_t
     return (
         scope_e.Dynamic if self.exec_opts.dynamic_scope() else
         scope_e.LocalOrGlobal
     )
 
+  def DynamicOrLocalOnly(self):
+    # type: () -> scope_t
+    return (
+        scope_e.Dynamic if self.exec_opts.dynamic_scope() else
+        scope_e.LocalOnly
+    )
+
   def ClearFlag(self, name, flag):
     # type: (str, int) -> bool
     """Used for export -n.
 
     We don't use SetValue() because even if rval is None, it will make an Undef
     value in a scope.
     """
-    cell, name_map = self._ResolveNameOnly(name, self._LookupMode())
+    cell, name_map = self._ResolveNameOnly(name, self.DynamicOrLocalGlobal())
     if cell:
       if flag & ClearExport:
         cell.exported = False
@@ -1843,13 +1850,27 @@ def GetMatch(self, i):
       return None
 
 
-def SetVar(mem, lval, val, flags=0):
+def SetVarShopt(mem, lval, val, flags=0):
   # type: (Mem, lvalue_t, value_t, int) -> None
-  """Equivalent of x=$y or setvar x = y 
+  """ Like 'setvar', unless dynamic scope is on.
   
-  Both of these respects shopt --unset dynamic_scope.
+  setvar <=> scope_e.LocalOrGlobal
+
+  Respects shopt --unset dynamic_scope.
+  """
+  lookup_mode = mem.DynamicOrLocalGlobal()
+  mem.SetValue(lval, val, lookup_mode, flags=flags)
+
+
+def SetLocalShopt(mem, lval, val, flags=0):
+  # type: (Mem, lvalue_t, value_t, int) -> None
+  """ Like 'setlocal', unless dynamic scope is on.
+  
+  setlocal <=> scope_e.LocalOnly
+
+  Respects shopt --unset dynamic_scope.
   """
-  lookup_mode = mem._LookupMode()
+  lookup_mode = mem.DynamicOrLocalOnly()
   mem.SetValue(lval, val, lookup_mode, flags=flags)
 
 

doc/variable-scope.md

@@ -112,26 +112,18 @@ The `setlocal` key always does the same thing.  but all these other constructs
 
 - Mutates exactly one scope!
 
-
-We introduce three names here: `setvar`, `setref`, and `setlocal`.
-
-And they will be used below.
-
-**Important**: They are both **keywords** and semantics.   For example, we say
-that all of these have `setvar` semantics:
-
-    x=y                  # shell assignment
-    : ${x=default}       # another form of shell assignment
-    setvar x = 'y'       # Oil keyword
-
 ## Where Are These Semantics Used?
 
 ### `Dynamic` → `LocalOrGlobal` (keyword `setvar`)
 
 Shell:
 
 - `x=y`
-- `export` too
+  - including: `s+=suffix`, `a[i]+=suffix`, `a+=(suffix 2)`.
+- `export x=y`
+- `readonly x=y`
+
+<!-- note: can all of these be LocalOnly?  It is possible in theory.  -->
 
 New Oil keyword: `setvar`
 
@@ -152,18 +144,10 @@ The other ones deal with values.  These deal with cells.  These also change to
 
 These shell constructs mutate.
 
-- `s+=suffix`, `a[i]+=suffix`, `a+=(suffix 2)`
-- `(( i = j ))`, `(( i += j ))`
-- `(( a[i] = j ))`, `(( a[i] += j ))`
-- `${undef=default}` and `${undef:=default}`
-- `myprog {fd}>out.txt`
-- `export`
-
-TODO:
-
-- These constructs can all still mutate globals.
-- Maybe what we should do is set ALL of them to SetLocalOrDynamic.  Except for
-  SetVar.
+- osh/word_eval: `${undef=default}` and `${undef:=default}`
+- core/process: `myprog {fd}>out.txt`
+- osh/sh_expr_eval: `(( i = j ))`, `(( i += j ))`
+  - `(( a[i] = j ))`, `(( a[i] += j ))`
 
 ### Unchanged: Builtins That Take "Out Params" (keyword `setref`)
 
@@ -177,12 +161,7 @@ Example.
 - `mapfile` / `readarray`
 - `printf -v`
 - `run --assign-status`
-
-TODO: Fix `unset`.
-
-- `unset` -- this takes a var name, so maybe it should be `setref`, not
-  `setvar`?
-  - it's really `unsetref`?
+- `unset` -- This takes a variable name, so it's like an "out param".
 
 ## More Details
 

osh/builtin_assign.py

@@ -79,7 +79,7 @@ def _PrintVariables(mem, cmd_val, attrs, print_flags, builtin=_OTHER):
   elif flag_g:
     lookup_mode = scope_e.GlobalOnly
   else:
-    lookup_mode = mem._LookupMode()
+    lookup_mode = mem.DynamicOrLocalGlobal()
 
   if len(cmd_val.pairs) == 0:
     print_all = True
@@ -228,8 +228,8 @@ def Run(self, cmd_val):
     else:
       for pair in cmd_val.pairs:
         # NOTE: when rval is None, only flags are changed
-        state.SetVar(self.mem, lvalue.Named(pair.var_name), pair.rval,
-                     flags=state.SetExport)
+        state.SetVarShopt(self.mem, lvalue.Named(pair.var_name), pair.rval,
+                          flags=state.SetExport)
 
     return 0
 
@@ -293,8 +293,8 @@ def Run(self, cmd_val):
       # NOTE:
       # - when rval is None, only flags are changed
       # - dynamic scope because flags on locals can be changed, etc.
-      state.SetVar(self.mem, lvalue.Named(pair.var_name), rval,
-                   flags=state.SetReadOnly)
+      state.SetVarShopt(self.mem, lvalue.Named(pair.var_name), rval,
+                        flags=state.SetReadOnly)
 
     return 0
 
@@ -397,7 +397,8 @@ def Run(self, cmd_val):
             rval = value.AssocArray({})
 
       rval = _ReconcileTypes(rval, arg.a, arg.A, pair.spid)
-      self.mem.SetValue(lvalue.Named(pair.var_name), rval, lookup_mode, flags=flags)
+      self.mem.SetValue(lvalue.Named(pair.var_name), rval, lookup_mode,
+                        flags=flags)
 
     return status
 

osh/cmd_eval.py

@@ -753,10 +753,10 @@ def _Dispatch(self, node, pipeline_st):
             elif case2(Id.KW_SetGlobal):
               lookup_mode = scope_e.GlobalOnly
             elif case2(Id.KW_SetRef):
-              # So this can modify two levels up?
+              # TODO: setref upvalue = 'returned'
+              # Require the cell.nameref flag on it.
               lookup_mode = scope_e.Dynamic
 
-              # TODO: setref upvalue = 'returned'
               e_die("setref isn't implemented")
             else:
               raise AssertionError(node.keyword.id)
@@ -820,7 +820,9 @@ def _Dispatch(self, node, pipeline_st):
       elif case(command_e.ShAssignment):  # Only unqualified assignment
         node = cast(command__ShAssignment, UP_node)
 
-        lookup_mode = self.mem._LookupMode()  # x=y is equivalent of setvar
+        # x=y is equivalent of setvar
+        lookup_mode = self.mem.DynamicOrLocalGlobal()
+
         for pair in node.pairs:
           spid = pair.spids[0]  # Source location for tracing
           # Use the spid of each pair.
@@ -1064,7 +1066,7 @@ def _Dispatch(self, node, pipeline_st):
           for x in iter_list:
             #log('> ForEach setting %r', x)
             self.mem.SetValue(lvalue.Named(iter_name), value.Str(x),
-                            scope_e.LocalOnly)
+                              scope_e.LocalOnly)
             #log('<')
 
             try:
@@ -1138,8 +1140,8 @@ def _Dispatch(self, node, pipeline_st):
               loop_val = it.next()
             except StopIteration:
               break
-            self.mem.SetValue(lvalue.Named(iter_name), _PyObjectToVal(loop_val),
-                            scope_e.LocalOnly)
+            self.mem.SetValue(lvalue.Named(iter_name),
+                              _PyObjectToVal(loop_val), scope_e.LocalOnly)
 
             # Copied from above
             try:

osh/sh_expr_eval.py

@@ -366,7 +366,7 @@ def _EvalLhsAndLookupArith(self, node):
   def _Store(self, lval, new_int):
     # type: (lvalue_t, int) -> None
     val = value.Str(str(new_int))
-    state.SetVar(self.mem, lval, val)
+    state.SetLocalShopt(self.mem, lval, val)
 
   def EvalToInt(self, node):
     # type: (arith_expr_t) -> int

osh/word_eval.py

@@ -567,8 +567,7 @@ def _ApplyTestOp(self,
               else: 
                 raise AssertionError()
 
-          # like setvar, not setref
-          state.SetVar(self.mem, lval, value.Str(rhs_str))
+          state.SetLocalShopt(self.mem, lval, value.Str(rhs_str))
         return True
 
       else:

spec/oil-scope.test.sh

@@ -160,7 +160,7 @@ x=
 ---
 x=
 x=
-x=new
+x=
 ## END
 
 #### declare -p respects it
@@ -230,3 +230,33 @@ x=X y=Y
 x= y=
 ## END
 
+
+#### SetLocalShopt constructs
+
+f() {
+  (( x = 42 ))
+}
+demo() {
+  f
+  echo x=$x
+}
+
+demo
+
+echo ---
+
+shopt --unset dynamic_scope
+
+unset x
+
+demo
+
+echo --- global
+echo x=$x
+## STDOUT:
+x=42
+---
+x=
+--- global
+x=
+## END