Repository: hylo-lang/hylo (swift)

Issue:
Issue with the `inout` subscript of `BitArray` in the stdlib
Issue occurred when trying to use the `inout` part of `BitArray` 's subscript implementation.
https://github.com/hylo-lang/hylo/blob/6a54950e2ce41ca835f5e9d3aeddc3d76a74307a/StandardLibrary/Sources/BitArray.hylo#L43-L48

I tried this:
```hylo
  var b = BitArray()
  &b.append(true)
  &b[0] = false
  precondition(b[0] == false)
```
... but it fails with:
```
error: cannot call value of type 'BitArray' as a subscript
  &b[0] = false
  ~~
```

Pull request:
Fixes #1351

The lowering of subscript didn't properly handled mutation. Fixing it surfaced a few missing mutation markers in the standard library as well as a puzzling use of a local `inout` binding.

@lucteo it would be good if you had a look at 6b0e524a92502e0e80aa2576d3126df60c0d367b and tell if the refactoring looks OK.

Patch:
diff --git a/Sources/IR/Emitter.swift b/Sources/IR/Emitter.swift
index f08b54729..2b9057928 100644
--- a/Sources/IR/Emitter.swift
+++ b/Sources/IR/Emitter.swift
@@ -1068,7 +1068,7 @@ struct Emitter {
 
   private mutating func emit(assignStmt s: AssignStmt.ID) -> ControlFlow {
     // The LHS should must be marked for mutation even if the statement denotes initialization.
-    guard program[s].left.kind == InoutExpr.self else {
+    guard ast.isMarkedForMutation(ast[s].left) else {
       let p = program[s].left.site.start
       report(.error(assignmentLHSRequiresMutationMarkerAt: .empty(at: p)))
       return .next
@@ -1575,23 +1575,22 @@ struct Emitter {
       }
     }
 
-    // Explicit arguments are evaluated first, from left to right.
-    let explicitArguments = emitArguments(
+    // Arguments are evaluated first, from left to right; callee and captures are evaluated next
+    let arguments = emitArguments(
       to: ast[e].callee, in: CallID(e),
       usingExplicit: ast[e].arguments, synthesizingDefaultAt: .empty(at: ast[e].site.end))
-
-    // Callee and captures are evaluated next.
-    let (callee, captures) = emitFunctionCallee(ast[e].callee, markedForMutation: false)
-    let arguments = captures + explicitArguments
+    let m = ast.isMarkedForMutation(ast[e].callee)
+    let (callee, captures) = emitFunctionCallee(ast[e].callee, markedForMutation: m)
+    let inputs = captures + arguments
 
     // Call is evaluated last.
     switch callee {
     case .direct(let r):
-      emitApply(.constant(r), to: arguments, writingResultTo: storage, at: ast[e].site)
+      emitApply(.constant(r), to: inputs, writingResultTo: storage, at: ast[e].site)
     case .lambda(let r):
-      emitApply(r, to: arguments, writingResultTo: storage, at: ast[e].site)
+      emitApply(r, to: inputs, writingResultTo: storage, at: ast[e].site)
     case .bundle(let r):
-      emitApply(r, to: arguments, writingResultTo: storage, at: ast[e].site)
+      emitApply(r, to: inputs, writingResultTo: storage, at: ast[e].site)
     }
   }
 
@@ -2063,15 +2062,13 @@ struct Emitter {
   private mutating func emitOperands(
     _ e: SubscriptCallExpr.ID
   ) -> (callee: BundleReference<SubscriptDecl>, arguments: [Operand]) {
-    // Explicit arguments are evaluated first, from left to right.
-    let explicitArguments = emitArguments(
+    // Arguments are evaluated first, from left to right; callee and captures are evaluated next.
+    let arguments = emitArguments(
       to: ast[e].callee, in: CallID(e),
       usingExplicit: ast[e].arguments, synthesizingDefaultAt: .empty(at: ast[e].site.end))
-
-    // Callee and captures are evaluated next.
-    let (callee, captures) = emitSubscriptCallee(ast[e].callee, markedForMutation: false)
-
-    return (callee, captures + explicitArguments)
+    let m = ast.isMarkedForMutation(ast[e].callee)
+    let (callee, captures) = emitSubscriptCallee(ast[e].callee, markedForMutation: m)
+    return (callee, captures + arguments)
   }
 
   /// Inserts the IR for infix operand `e` passed with convention `access`.
@@ -2152,6 +2149,7 @@ struct Emitter {
       return (.direct(f), [])
 
     case .member(let d, _, _) where d.isCallable:
+      // Callee is a member reference; the receiver is the only capture.
       return emitMemberFunctionCallee(callee, markedForMutation: isMutating)
 
     case .builtinFunction, .builtinType:
@@ -2165,35 +2163,28 @@ struct Emitter {
     }
   }
 
-  /// Inserts the IR evaluating `callee`, is a reference to a member function marked for mutation
-  /// iff `isMutating` is `true`, the callee's value along with the call receiver.
+  /// Inserts the IR evaluating `callee`, which refers to a member function marked for mutation
+  /// iff `isMutating` is `true`, returning the callee's value along with the call receiver.
   private mutating func emitMemberFunctionCallee(
     _ callee: NameExpr.ID, markedForMutation isMutating: Bool
   ) -> (callee: Callee, captures: [Operand]) {
-    guard case .member(let d, let a, let s) = program[callee].referredDecl else {
-      unreachable()
-    }
+    guard case .member(let d, let a, let s) = program[callee].referredDecl else { unreachable() }
 
     let receiver = emitLValue(receiver: s, at: ast[callee].site)
     let receiverType = module.type(of: receiver).ast
 
-    let available = receiverCapabilities(program[callee].type)
-    let requested = available.intersection(
-      AccessEffectSet.forUseOfBundle(performingInPlaceMutation: isMutating))
-
-    // TODO: requested is empty iff the program is ill-typed w.r.t. mutation markers
-    // assert(!requested.isEmpty)
-    let k = requested.isEmpty ? available : requested
-
-    let functionToCall = module.memberCallee(
-      referringTo: d, memberOf: receiverType, accessedWith: k,
+    let request = program.requestedCapabilities(
+      onBundleProviding: receiverCapabilities(program[callee].type),
+      forInPlaceMutation: isMutating)
+    let entityToCall = module.memberCallee(
+      referringTo: d, memberOf: receiverType, accessedWith: request,
       specializedBy: a, usedIn: program[callee].scope)
 
-    if case .bundle(let b) = functionToCall {
+    if case .bundle(let b) = entityToCall {
       return emitMethodBundleCallee(referringTo: b, on: receiver, at: program[callee].site)
     } else {
-      let c = insert(module.makeAccess(k, from: receiver, at: program[callee].site))!
-      return (callee: functionToCall, captures: [c])
+      let c = insert(module.makeAccess(request, from: receiver, at: program[callee].site))!
+      return (callee: entityToCall, captures: [c])
     }
   }
 
@@ -2246,7 +2237,7 @@ struct Emitter {
     case InoutExpr.self:
       return emitSubscriptCallee(ast[InoutExpr.ID(callee)!].subject, markedForMutation: true)
     default:
-      UNIMPLEMENTED()
+      UNIMPLEMENTED("call to an anonymous subscript of an rvalue")
     }
   }
 
@@ -2258,25 +2249,17 @@ struct Emitter {
     switch program[callee].referredDecl {
     case .direct(let d, let a) where d.kind == SubscriptDecl.self:
       // Callee is a direct reference to a subscript declaration.
-      let t = SubscriptType(canonical(program[d].type))!
-      guard t.environment == .void else {
-        UNIMPLEMENTED()
+      guard SubscriptType(canonical(program[d].type))!.environment == .void else {
+        UNIMPLEMENTED("subscript with non-empty environment")
       }
 
-      let b = BundleReference(
-        to: SubscriptDecl.ID(d)!, specializedBy: a, requesting: t.capabilities)
-      return (b, [])
-
-    case .member(let d, let a, let s) where d.kind == SubscriptDecl.self:
-      // Callee is a member reference to a subscript declaration.
-      let t = SubscriptType(canonical(program[d].type))!
-      let b = BundleReference(
-        to: SubscriptDecl.ID(d)!, specializedBy: a, requesting: t.capabilities)
+      let entityToCall = program.subscriptBundleReference(
+        to: .init(d)!, specializedBy: a, markedForMutation: isMutating)
+      return (entityToCall, [])
 
-      // The callee's receiver is the sole capture.
-      let receiver = emitLValue(receiver: s, at: ast[callee].site)
-      let i = insert(module.makeAccess(t.capabilities, from: receiver, at: ast[callee].site))!
-      return (b, [i])
+    case .member(let d, _, _) where d.kind == SubscriptDecl.self:
+      // Callee is a member reference; the receiver is the only capture.
+      return emitMemberSubscriptCallee(callee, markedForMutation: isMutating)
 
     case .builtinFunction, .builtinType:
       // There are no built-in subscripts.
@@ -2287,6 +2270,20 @@ struct Emitter {
     }
   }
 
+  /// Inserts the IR evaluating `callee`, which refers to a member subscript marked for mutation
+  /// iff `isMutating` is `true`, returning the callee's value along with the call receiver.
+  private mutating func emitMemberSubscriptCallee(
+    _ callee: NameExpr.ID, markedForMutation isMutating: Bool
+  ) -> (callee: BundleReference<SubscriptDecl>, captures: [Operand]) {
+    guard case .member(let d, let a, let s) = program[callee].referredDecl else { unreachable() }
+
+    let entityToCall = program.subscriptBundleReference(
+      to: .init(d)!, specializedBy: a, markedForMutation: isMutating)
+    let r = emitLValue(receiver: s, at: ast[callee].site)
+    let c = insert(module.makeAccess(entityToCall.capabilities, from: r, at: ast[callee].site))!
+    return (entityToCall, [c])
+  }
+
   /// Returns `(success: a, failure: b)` where `a` is the basic block reached if all items in
   /// `condition` hold and `b` is the basic block reached otherwise, creating new basic blocks
   /// in `scope`.
@@ -2657,9 +2654,9 @@ struct Emitter {
 
   /// Inserts the IR for lvalue `e`.
   private mutating func emitLValue(_ e: SubscriptCallExpr.ID) -> Operand {
-    let (callee, arguments) = emitOperands(e)
+    let (b, a) = emitOperands(e)
     let s = module.makeProjectBundle(
-      applying: .init(callee, in: insertionScope!), to: arguments, at: ast[e].site)
+      applying: b, to: a, at: ast[e].site, canonicalizingTypesIn: insertionScope!)
     return insert(s)!
   }
 
@@ -2772,7 +2769,8 @@ struct Emitter {
     let b = BundleReference(to: d, specializedBy: specialization, requesting: t.capabilities)
     let r = insert(module.makeAccess(t.capabilities, from: receiver, at: site))!
 
-    let s = module.makeProjectBundle(applying: .init(b, in: insertionScope!), to: [r], at: site)
+    let s = module.makeProjectBundle(
+      applying: b, to: [r], at: site, canonicalizingTypesIn: insertionScope!)
     return insert(s)!
   }
 
diff --git a/Sources/IR/Operands/Instruction/ProjectBundle.swift b/Sources/IR/Operands/Instruction/ProjectBundle.swift
index 3cff64b72..5c0d5e7ed 100644
--- a/Sources/IR/Operands/Instruction/ProjectBundle.swift
+++ b/Sources/IR/Operands/Instruction/ProjectBundle.swift
@@ -71,27 +71,30 @@ extension ProjectBundle: CustomStringConvertible {
 
 extension Module {
 
-  /// Creates a `project_bundle` anchored at `site` that projects a value by applying one of the
-  /// variants in `bundle` on `arguments`.
+  /// Creates a `project_bundle` anchored at `site` that applies one of the variants defined in `m`
+  /// to arguments `a`, canonicalizing types in `scopeOfUse`.
   mutating func makeProjectBundle(
-    applying bundle: ScopedValue<BundleReference<SubscriptDecl>>,
-    to arguments: [Operand],
-    at site: SourceRange
+    applying m: BundleReference<SubscriptDecl>, to a: [Operand],
+    at site: SourceRange,
+    canonicalizingTypesIn scopeOfUse: AnyScopeID
   ) -> ProjectBundle {
     var variants: [AccessEffect: Function.ID] = [:]
-    for v in program[bundle.value.bundle].impls {
-      variants[program[v].introducer.value] = demandDeclaration(lowering: v)
+    for v in program[m.bundle].impls {
+      let i = program[v].introducer.value
+      if m.capabilities.contains(i) {
+        variants[program[v].introducer.value] = demandDeclaration(lowering: v)
+      }
     }
+    precondition(!variants.isEmpty)
 
-    let bundleType = program.canonicalType(
-      of: bundle.value.bundle, specializedBy: bundle.value.arguments, in: bundle.scope)
-    let t = SubscriptType(bundleType)!.pure
+    let t = SubscriptType(
+      program.canonicalType(of: m.bundle, specializedBy: m.arguments, in: scopeOfUse))!.pure
 
     return .init(
-      bundle: bundle.value, variants: variants,
+      bundle: m, variants: variants,
       parameters: t.inputs.lazy.map({ ParameterType($0.type)! }),
       projection: RemoteType(t.output)!.bareType,
-      operands: arguments, site: site)
+      operands: a, site: site)
   }
 
 }
diff --git a/Sources/IR/TypedProgram+Extensions.swift b/Sources/IR/TypedProgram+Extensions.swift
index bfdbc9938..d44cd30b2 100644
--- a/Sources/IR/TypedProgram+Extensions.swift
+++ b/Sources/IR/TypedProgram+Extensions.swift
@@ -29,4 +29,28 @@ extension TypedProgram {
     }
   }
 
+  /// Returns a subscript bundle reference to `d`, which occurs specialized by `z` and is marked
+  /// for mutation iff `isMutating` is `true`.
+  func subscriptBundleReference(
+    to d: SubscriptDecl.ID, specializedBy z: GenericArguments, markedForMutation isMutating: Bool
+  ) -> BundleReference<SubscriptDecl> {
+    let t = SubscriptType(canonical(self[d].type, in: self[d].scope))!
+    let r = requestedCapabilities(
+      onBundleProviding: t.capabilities, forInPlaceMutation: isMutating)
+    return BundleReference(to: d, specializedBy: z, requesting: r)
+  }
+
+  /// Returns the capabilities potentially requested by an access on a subscript or method bundle
+  /// defining `available`, used for mutation iff `m` is `true`.
+  func requestedCapabilities(
+    onBundleProviding available: AccessEffectSet, forInPlaceMutation m: Bool
+  ) -> AccessEffectSet {
+    let requested = available.intersection(
+      AccessEffectSet.forUseOfBundle(performingInPlaceMutation: m))
+
+    // TODO: requested is empty iff the program is ill-typed w.r.t. mutation markers
+    // assert(!requested.isEmpty)
+    return requested.isEmpty ? available : requested
+  }
+
 }
diff --git a/StandardLibrary/Sources/Array.hylo b/StandardLibrary/Sources/Array.hylo
index 2bed1694a..c3d7a71d7 100644
--- a/StandardLibrary/Sources/Array.hylo
+++ b/StandardLibrary/Sources/Array.hylo
@@ -248,7 +248,7 @@ public conformance Array: Collection {
     }
     inout {
       // precondition(position >= 0 && position < count(), "position is out of bounds")
-      pointer_to_element[at: position].unsafe[]
+      &pointer_to_element[at: position].unsafe[]
     }
   }
 
diff --git a/StandardLibrary/Sources/BitArray.hylo b/StandardLibrary/Sources/BitArray.hylo
index d447891fe..449b475b8 100644
--- a/StandardLibrary/Sources/BitArray.hylo
+++ b/StandardLibrary/Sources/BitArray.hylo
@@ -54,9 +54,9 @@ public type BitArray {
   fun set_value(_ b: Bool, for p: Position) inout {
     let m = 1 << p.offset
     if b {
-      bits[p.bucket] |= m
+      &bits[p.bucket] |= m
     } else {
-      bits[p.bucket] &= ~m
+      &bits[p.bucket] &= ~m
     }
   }
 
@@ -126,4 +126,4 @@ public conformance BitArray.Position: Regular {
     (self.bucket == other.bucket) && (self.offset == other.offset)
   }
 
-}
\ No newline at end of file
+}
diff --git a/StandardLibrary/Sources/Concurrency/Future.hylo b/StandardLibrary/Sources/Concurrency/Future.hylo
index d82e5ec3f..10834ca59 100644
--- a/StandardLibrary/Sources/Concurrency/Future.hylo
+++ b/StandardLibrary/Sources/Concurrency/Future.hylo
@@ -20,8 +20,8 @@ public type Future<E: Movable & Deinitializable> {
     &self.r = .none()
 
     let local_f = fun (_ frame: inout SpawnFrameBase) -> Void {
-      inout this = PointerToMutable<Self>(type_punning: mutable_pointer[to: &frame]).unsafe[]
-      &this.do_call()
+      var this = PointerToMutable<Self>(type_punning: mutable_pointer[to: &frame])
+      &this.unsafe[].do_call()
     }
     concore2full_spawn2(&self.base_frame, local_f)
   }
diff --git a/StandardLibrary/Sources/DynamicBuffer.hylo b/StandardLibrary/Sources/DynamicBuffer.hylo
index 5e1ee5104..db8c2e9f8 100644
--- a/StandardLibrary/Sources/DynamicBuffer.hylo
+++ b/StandardLibrary/Sources/DynamicBuffer.hylo
@@ -79,7 +79,7 @@ public type DynamicBuffer<Header: Deinitializable, Element>: Deinitializable, Mo
   /// - Requires: `capacity() > 0`.
   public property header: Header {
     let { buffer_header().unsafe[].1 }
-    inout { &buffer_header().unsafe[].1 }
+    inout { &(buffer_header()).unsafe[].1 }
   }
 
   /// Returns the address of the `self`'s header.


Test patch:
diff --git a/Tests/LibraryTests/TestCases/BitArrayTests.hylo b/Tests/LibraryTests/TestCases/BitArrayTests.hylo
new file mode 100644
index 000000000..70098a18f
--- /dev/null
+++ b/Tests/LibraryTests/TestCases/BitArrayTests.hylo
@@ -0,0 +1,12 @@
+//- compileAndRun expecting: success
+
+fun test_modify() {
+  var b = BitArray()
+  &b.append(true)
+  &b[0] = false
+  precondition(!b[0])
+}
+
+public fun main() {
+  test_modify()
+}