[Bug] Returning an unmanaged class results in a retain

[jtenner]

@unmanaged
class ExampleClass {}

function returnExample(): ExampleClass {
  return new ExampleClass();
}

let t = returnExample();

 (func $start (; 13 ;) (type $FUNCSIG$v)
  (local $0 i32)
  call $~lib/rt/tlsf/__alloc
  local.tee $0
  i32.const 160
  i32.gt_u
  if
   local.get $0
   i32.const 16
   i32.sub
   call $~lib/rt/pure/increment
  end
  local.get $0
  global.set $rc/unmanaged-return/t
 )

It's not accounting for the @unmanaged Example class.

[jtenner]

16
Error
    at r.compileCallDirect (C:\Users\jtenner\Desktop\projects\assemblyscript\dist\assemblyscript.js:1:293524)
    at r.compileCallExpression (C:\Users\jtenner\Desktop\projects\assemblyscript\dist\assemblyscript.js:1:288764)
    at r.compileExpression (C:\Users\jtenner\Desktop\projects\assemblyscript\dist\assemblyscript.js:1:245513)
    at r.compileVariableStatement (C:\Users\jtenner\Desktop\projects\assemblyscript\dist\assemblyscript.js:1:240872)

Looks like it's coming from the variableStatement.

        initExpr = this.compileExpression(declaration.initializer, Type.auto,
          Constraints.WILL_RETAIN
        ); // reports
        initAutoreleaseSkipped = this.skippedAutoreleases.has(initExpr);

How do you determine if an Expression is managed?

[dcodeIO]

The compiler should only insert RT calls if relevantType.isManaged (e.g. instance.signature.returnType.isManaged if a call). Maybe this check is missing somewhere.

[jtenner]

I'm trying to trace it. It's very hard.

[jtenner]

I hate to be a bother, but I did find another compiler bug for sure.

@unmanaged
class ExampleClass {}

static report<T>(actual: T): void {
    // get the stack trace
    Actual.stackTrace = getStackTrace();

    // if T is a reference type...
    if (isReference<T>()) {

      if (isNullable<T>()) {
        if (actual === null) {
          Actual.type = ValueType.Null;
          return;
        }
      }

      let ptr = changetype<usize>(actual);

      // if the reference is managed, retain it for later reporting
      if (isManaged<T>()) __retain(ptr);

I accounted for this with the fixed and rebuilt compiler to make sure that my testing software wasn't retaining the reference directly.

(func $../cli/node_modules/@as-pect/assembly/assembly/internal/report/Actual/Actual.report<assembly/__tests__/unmanaged.spec/ExampleClass> (; 59 ;) (type $FUNCSIG$vi) (param $0 i32)
  (local $1 i32)
  call $../cli/node_modules/@as-pect/assembly/assembly/internal/report/Actual/getStackTrace
  global.set $../cli/node_modules/@as-pect/assembly/assembly/internal/report/Actual/Actual.stackTrace
  local.get $0
  local.set $1
  local.get $1
  call $~lib/rt/pure/__retain
  drop

This is unoptimized output, but the last 5 instructions get the parameter and then retain it immediately after setting the ptr variable.

I'll work on reproducing a smaller concise example.

[jtenner]

The first if statement passed fine. It looks like the resolver isn't picking up that T is unmanaged.

[dcodeIO]

The assertion is from not providing T when calling testIsManaged I think. That's a different issue.

[jtenner]

Actually you're right. I'm forgetting the generic parameter in the example. For some reason, however, the initial example I provided seems to call __retain.

[jtenner]

Yeah. The problem was on me double-fold. I had to build the compiler from source to test and validate that it worked correctly. Hopefully, dist files can be updated soon.