Dart_New cannot allocate using redirecting factory constructors in abstract classes

[bkonyi]

FAILED: dartk-vm debug_x64 vm/cc/DartAPI_New
Expected: Pass
Actual: Fail

--- Command "run_vm_unittest" (took 978ms):
DART_CONFIGURATION=DebugX64 out/DebugX64/run_vm_tests --dfe=out/DebugX64/gen/kernel-service.dart.snapshot --ignore-unrecognized-flags --use-dart-frontend DartAPI_New

exit code:
255

stdout:
Running test: DartAPI_New
Done: DartAPI_New

stderr:
../../runtime/vm/dart_api_impl_test.cc: 4733: error: expected 'result' to be a valid handle but found an error handle:
    'Dart_New: could not find constructor 'MyInterface.named'.'

../../runtime/vm/dart_api_impl_test.cc: 4734: error: expected 'Dart_ObjectIsType(result, type, &instanceOf)' to be a valid handle but found an error handle:
    'Dart_New: could not find constructor 'MyInterface.named'.'

../../runtime/vm/dart_api_impl_test.cc: 4735: error: expected: instanceOf
../../runtime/vm/dart_api_impl_test.cc: 4738: error: expected 'Dart_IntegerToInt64(foo, &int_value)' to be a valid handle but found an error handle:
    'Dart_New: could not find constructor 'MyInterface.named'.'

../../runtime/vm/dart_api_impl_test.cc: 4739: error: expected: <11> but was: <0>
../../runtime/vm/dart_api_impl_test.cc: 4743: error: expected 'result' to be a valid handle but found an error handle:
    'Dart_New: could not find constructor 'MyInterface.multiply'.'

../../runtime/vm/dart_api_impl_test.cc: 4744: error: expected 'Dart_ObjectIsType(result, type, &instanceOf)' to be a valid handle but found an error handle:
    'Dart_New: could not find constructor 'MyInterface.multiply'.'

../../runtime/vm/dart_api_impl_test.cc: 4745: error: expected: instanceOf
../../runtime/vm/dart_api_impl_test.cc: 4748: error: expected 'Dart_IntegerToInt64(foo, &int_value)' to be a valid handle but found an error handle:
    'Dart_New: could not find constructor 'MyInterface.multiply'.'

../../runtime/vm/dart_api_impl_test.cc: 4749: error: expected: <1100> but was: <0>

--- Re-run this test:
python tools/test.py -m debug -c dartk -r vm vm/cc/DartAPI_New

[bkonyi]

@peter-ahe-google, is this related to #29841? This is the kernel I'm seeing:

main = tmp::main;                                                                                                                                                                                                                       
library from "file:///usr/local/google/home/bkonyi/sdk/tmp.dart" as tmp {                                                                                                                                                               
  class MyClass extends core::Object {                                                                                                                                                                                                  
    field dynamic foo;                                                                                                                                                                                                                  
    constructor •() → void                                                                                                                                                                                                              
      : tmp::MyClass::foo = 7, super core::Object::•() {}                                                                                                                                                                               
    constructor named(dynamic value) → void                                                                                                                                                                                             
      : tmp::MyClass::foo = value, super core::Object::•() {}                                                                                                                                                                           
    constructor _hidden(dynamic value) → void                                                                                                                                                                                           
      : tmp::MyClass::foo = value.unary-(), super core::Object::•() {}                                                                                                                                                                  
    constructor exception(dynamic value) → void                                                                                                                                                                                         
      : tmp::MyClass::foo = value, super core::Object::•() {                                                                                                                                                                            
      throw "ConstructorDeath";                                                                                                                                                                                                         
    }                                                                                                                                                                                                                                   
    static factory multiply(dynamic value) → tmp::MyClass {                                                                                                                                                                             
      return new tmp::MyClass::named(value.*(100));                                                                                                                                                                                     
    }                                                                                                                                                                                                                                   
    static factory nullo() → tmp::MyClass {                                                                                                                                                                                             
      return null;                                                                                                                                                                                                                      
    }                                                                                                                                                                                                                                   
  }                                                                                                                                                                                                                                     
  abstract class MyExtraHop extends core::Object {                                                                                                                                                                                      
    static field dynamic _redirecting# = <dynamic>[tmp::MyExtraHop::hop];                                                                                                                                                               
    static factory hop(dynamic value) → tmp::MyExtraHop                                                                                                                                                                                 
      let dynamic #redirecting_factory = tmp::MyClass::named in invalid-expression;                                                                                                                                                     
  }                                                                                                                                                                                                                                     
  abstract class MyInterface extends core::Object {                                                                                                                                                                                     
    static field dynamic _redirecting# = <dynamic>[tmp::MyInterface::named, tmp::MyInterface::multiply];                                                                                                                                
    constructor notfound(dynamic value) → void                                                                                                                                                                                          
      : super core::Object::•()                                                                                                                                                                                                         
      ;                                                                                                                                                                                                                                 
    static factory named(dynamic value) → tmp::MyInterface                                                                                                                                                                              
      let dynamic #redirecting_factory = tmp::MyExtraHop::hop in invalid-expression;                                                                                                                                                    
    static factory multiply(dynamic value) → tmp::MyInterface                                                                                                                                                                           
      let dynamic #redirecting_factory = tmp::MyClass::multiply in invalid-expression;                                                                                                                                                  
  }                                                                                                                                                                                                                                     
  static method main() → dynamic {}                                                                                                                                                                                                     
}

[peter-ahe-google]

That looks like the same problem.

Basically, the way we represent redirecting factory constructors, they can't be invoked via any reflective API such as dart:mirrors and dart_api.h

Here's the location of the first error, line 4733 and here is the source code of the program.

/cc @stefantsov

[chloestefantsova]

The current status for redirecting factory constructors is the following. Redirecting factory constructors are represented using let-nodes and invalid-expressions. There exists a first-class representations for it, but fasta doesn't currently produce that because the patching mechanism should be fixed first (we need to patch procedures with redirecting factories and redirecting factories with procedures; currently, we can only patch members with the same kind of members). The work was deprioritized at some point, but I plan to get back to it soon.

After the work will be done, we'll have a representation that is easy to parse at the backends and resolve the final target of the invocation.

[bkonyi]

Any update on this @stefantsov ?

[chloestefantsova]

Unfortunately, I don't have an answer right now. I'm currently working on adding support for more compile-time errors related to type arguments. I plan to work on this issue in the next week.

[bkonyi]

Sorry to ping you again @stefantsov, but has there been any progress on this?

[chloestefantsova]

Not much of a progress @bkonyi, sorry. I did have a few discussions about the issue on the both sides: the front-end and the VM, and it looks like it would be better to fix the representation of redirecting factories issued by the compiler, so that the VM doesn't have to pattern-match on the how they are represented currently.

I've created an issue for the blocker: #33495. Could you please assign a priority to this issue as well as for the blocker?

[dgrove]

After talking with @bkonyi , moving to Dart 2.1 .

[a-siva]

Issue #33495 on which this issue is dependent on is not slated for a fix in Dart 2.1, moving the milestone of this issue to reflect that.

[bkonyi]

@a-siva, given this is blocked on #33495 it can probably be punted down the line to 2.4.

[bkonyi]

Just going to clear the milestone for this issue as there's no sign of it being resolved anytime soon.

@stefantsov do you know if there's any update on this?

[bkonyi]

cc @johnniwinther it looks like this is still an issue.

[johnniwinther]

Yes, we still need a fix for #33495