Why isn’t isUniquelyReferenced(_:) working here?

taylorswift · 2020-06-27T18:21:26.746Z

could we have something like a

enum E
{
    case foo(inout [Int], Bool)
}

where pattern matching it with a var binding would mutate the original enum payload?

that still wouldn’t solve the case where completely replacing the enum case should release all references though. but can’t that already be done without changing the semantics of the language?

anandabits · 2020-06-27T18:53:49.451Z

taylorswift:

could we have something like a
enum E
{
    case foo(inout [Int], Bool)
}

I don’t think it makes sense to put inout in the case declaration. It should be in the pattern match, like this:

var e = E.foo([42], true)
switch e {
case .e(inout ints, inout bool):
    ints.append(43)
    bool = false
}
print(e) // .foo([42, 43], false)

Lantua · 2020-06-27T18:59:43.632Z

It should also hold write access to e, so?

switch &e {
...
}

I wonder what should be the right behaviour if we access e in case block. It should be access violation, but then default case makes me uncertain.

taylorswift · 2020-06-27T19:46:31.757Z

Lantua:

I wonder what should be the right behaviour if we access e in case block. It should be access violation, but then default case makes me uncertain.

well, a switch is really just an elaborate if statement, so there’s no reason why the following should have different behavior, yet for some reason it does:

func foo()  
{
    var e:E = .s(.init())
    if case .s(var s) = e 
    {
        s.foo() // true 
        e = .empty 
        s.foo() // true
    }
    
    e = .s(.init())
    switch e 
    {
    case .s(var s):
        s.foo() // false
        e = .empty 
        s.foo() // false
    default:
        break
    }
}

taylorswift · 2020-06-27T19:48:08.983Z

anandabits:

I don’t think it makes sense to put inout in the case declaration. It should be in the pattern match, like this:

this would make for some strange spellings when combined with let/var bindings. where else in the language do you see inout var?

John_McCall · 2020-06-28T00:57:42.360Z

It wouldn’t make any sense to combine this with var or let.

Lantua is right, though: we’d want something up front in the switch to flag that the whole thing is a mutation of the variable. Once you have that, we could potentially just use var in the pattern itself.

Lantua · 2020-06-28T15:07:14.031Z

Syntax wise, I think mutating switch works nicely in tandem with inout variable declaration:

do {
  ref x = &y
}

switch &y {
case ref x: ...
}

taylorswift · 2020-06-28T19:06:54.428Z

Lantua:

Syntax wise, I think mutating switch works nicely in tandem with inout variable declaration:

John_McCall:

Lantua is right, though: we’d want something up front in the switch to flag that the whole thing is a mutation of the variable. Once you have that, we could potentially just use var in the pattern itself.

i would support the switch &y syntax. but i think this thread (as usual) has drifted far from the original issue, which is that this assignment:

case .c(var c):
    e = .empty

ought to decrement the reference count of c by 1, just as setting an equivalent Optional<C> to nil would. i don’t see why we need to invent new syntax and completely new semantics to fix what i would just have considered a bug.

John_McCall · 2020-06-28T19:17:32.256Z

It does, but the old enum value is copied to get the value to switch over, and then the enum payload is copied to initialize the var, so it’s still not uniquely referenced. This is why I emphasized the importance of doing an “in-place” switch that guarantees direct binding all the way down.

taylorswift · 2020-06-28T20:44:34.858Z

John_McCall:

but the old enum value is copied to get the value to switch over,

why does this retain persist through the entire execution of the case blocks? the reference is only needed for the pattern matching:

    e = .s(.init()) // +1 (total: 1)
    switch e // +1 (total: 2)
    {
    // +1 for var binding (total: +3)
    case .s(var s): 
    // -1 for return from pattern match (total: +2)
        s.foo() 
        e = .empty // -1 (total: +1)
        s.foo() // should be uniquely referenced
   
    default:
    // -1 for return from pattern match (total: +1)
        break
    }

John_McCall · 2020-06-28T21:19:25.649Z

It’s not guaranteed either way, but yes, the implementation could absolutely do a better job “forwarding” the switched-over value into the pattern variable.

taylorswift · 2020-06-28T21:25:43.572Z

should I file a bug?

John_McCall · 2020-06-29T16:27:45.399Z

Never hurts.

scanon · 2020-06-29T16:30:08.379Z

should I file a bug?

John already sorta said this, but the answer to "should I file a bug?" is always yes. If it makes sense to ask the question, the answer is yes. (Not just for Swift, for any project.)

taylorswift · 2020-06-29T17:48:30.062Z

Filed the following issues:

SR-13112: assignment to enum instance should deinitialize associated values on all optimization levels

SR-13113: assignment to enum instance should recursively deinitialize associated values

SR-13114: switch statement over enum instance should release references in associated values on successful pattern match

sighoya · 2020-07-02T11:07:35.415Z

  case .s(var s): 
   // -1 for return from pattern match (total: +2)
       s.foo() 
       e = .empty // -1 (total: +1)
      s.foo() // should be uniquely referenced

If s would be a struct which is owned by the enum, i.e. it is stored in the enum value, the above code would be quite problematic if the struct s would be referenced out of the enum and then the enum value is changed, making further access invalid.

To cicumvent the problem, the enum value fixed as switch param is duplicated behind the scenes s.t. replacing the enum value e inside the switch block would only mutate the enum value outside the switch block and not the duplicated enum value inside.

But, I don't know if these are the current semantics of Swift's switch.

taylorswift · 2020-07-02T18:29:39.384Z

sighoya:

To cicumvent the problem, the enum value fixed as switch param is duplicated behind the scenes s.t. replacing the enum value e inside the switch block would only mutate the enum value outside the switch block and not the duplicated enum value inside.

why would you expect the assignment to not mutate the enum value inside the case block?

sighoya · 2020-07-02T22:04:03.564Z

why would you expect the assignment to not mutate the enum value inside the case block?

I would expect it because the old subvalue s of the enum is accessed after mutation inducing undefined behavior (UB). To circumvent UB, a safe copy has to made before.

Nonetheless, it would be much clearer to deny any writeable operation to a enum value during referencing a subvalue of it.

taylorswift · 2020-07-02T22:24:32.470Z

sighoya:

I would expect it because the old subvalue s of the enum is accessed after mutation inducing undefined behavior (UB). To circumvent UB, a safe copy has to made before.

no, the var expression binds the associated value to a new value s, making it independent from the old value in the enum, which can be destroyed at any time without affecting the newly bound value.

sighoya · 2020-07-03T13:46:13.413Z

no, the var expression binds the associated value to a new value s , making it independent from the old value in the enum , which can be destroyed at any time without affecting the newly bound value.

Okay, looking amateurish at the SIL of your program, it seems that C.init() is separated from the enum such that a new hidden (stack?) var is created storing C.init() and var c just refers to this hidden value which is however not possible in more general cases.

Question:
Is it guaranteed that releasing values of aggregate type (release_value) also decrements references inside the aggregate value immediately?
If yes, then your case seems to be a bug.