dart-lang / sdk

The Dart SDK, including the VM, JS and Wasm compilers, analysis, core libraries, and more.
https://dart.dev
BSD 3-Clause "New" or "Revised" License
10.25k stars 1.58k forks source link

Hot reload fails on incremental kernel file due to redirecting factory #30875

Closed aam closed 7 years ago

aam commented 7 years ago

#redirecting_factory declaration below seems to be invalid(invalid-expression)

  abstract class GlobalKey<T extends fra2::State<fra2::StatefulWidget>> extends fra2::Key {
...
    static factory •<T extends fra2::State<fra2::StatefulWidget>>({core::String debugLabel}) → fra2::GlobalKey<fra2::GlobalKey::•::T>
      let dynamic #redirecting_factory = fra2::LabeledGlobalKey::_ in invalid-expression;

generated from https://github.com/flutter/flutter/blob/master/packages/flutter/lib/src/widgets/framework.dart#L178

abstract class GlobalKey<T extends State<StatefulWidget>> extends Key {
  factory GlobalKey({ String debugLabel }) = LabeledGlobalKey<T>._;
  const GlobalKey.constructor() : super._();

fails during hot reload in StreamingFlowGraphBuilder#BuildVariableDeclaration on #redirecting_factory.

[   +3 ms] I/DartVM  ( 3956): BuildGraphOfFunction dart_function: Function 'RecipeGridPage.': constructor const.
[        ] I/DartVM  ( 3956): BuildGraphOfFunction dart_function: Function 'createState':.
[        ] I/DartVM  ( 3956): BuildGraphOfFunction dart_function: Function '_RecipeGridPageState@349046805.': constructor.
[        ] I/DartVM  ( 3956): BuildGraphOfFunction dart_function: Function 'GlobalKey.': static factory.
[        ] I/DartVM  ( 3956): BuildVariableDeclaration name=#redirecting_factory
[        ] I/DartVM  ( 3956): BuildStaticGet unimplemented with target=_@39042623, function=Function: null
[        ] E/DartVM  ( 3956): ../../dart/runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc: 5501: error: unimplemented code
[        ] E/DartVM  ( 3956): Dumping native stack trace for thread fac
[        ] E/DartVM  ( 3956):   [0xd74730dd] Unknown symbol
[        ] E/DartVM  ( 3956):   [0xd74730dd] Unknown symbol
[        ] E/DartVM  ( 3956):   [0xd770a529] Unknown symbol
[        ] E/DartVM  ( 3956):   [0xd758dc7f] Unknown symbol
[        ] E/DartVM  ( 3956):   [0xd7589c3d] Unknown symbol
[        ] E/DartVM  ( 3956):   [0xd7593e87] Unknown symbol
[   +1 ms] E/DartVM  ( 3956):   [0xd7589ef3] Unknown symbol
[        ] E/DartVM  ( 3956):   [0xd758cae5] Unknown symbol
[        ] E/DartVM  ( 3956):   [0xd758c3f5] Unknown symbol
[        ] E/DartVM  ( 3956):   [0xd758cded] Unknown symbol
[        ] E/DartVM  ( 3956):   [0xd759f7d5] Unknown symbol
[        ] E/DartVM  ( 3956):   [0xd75bb625] Unknown symbol
[        ] E/DartVM  ( 3956):   [0xd75bdefd] Unknown symbol
[   +1 ms] E/DartVM  ( 3956):   [0xd75bf63b] Unknown symbol
[        ] E/DartVM  ( 3956):   [0xd75bf101] Unknown symbol
[        ] E/DartVM  ( 3956):   [0xd75bba49] Unknown symbol
[        ] E/DartVM  ( 3956): -- End of DumpStackTrace
[   +5 ms] F/libc    ( 3956): Fatal signal 6 (SIGABRT), code -

There seem to be no errors/warning produced for that Dart code

 $FH/engine/src/out/host_debug_unopt/dart-sdk/bin/dart $FH/engine/src/out/host_debug_unopt/gen/frontend_server.dart.snapshot --sdk-root $FH/engine/src/out/android_debug_unopt/flutter_patched_sdk --incremental $FH/flutter/examples/flutter_gallery/lib/main.dart                                                                                        
result 47b9fb8b-f2b6-4cfc-87f7-339ba117b0fe
Warning: line 1, column 11276 of ../../packages/flutter/lib/src/rendering/binding.dart: Superclass has no method named 'hitTest'.
Nit: line 1, column 229 of lib/demo/material/date_and_time_picker_demo.dart: Import of 'TextDirection' (from 'package:intl/intl.dart') hides import from 'dart:ui'.
Nit: line 1, column 271 of lib/demo/material/full_screen_dialog_demo.dart: Import of 'TextDirection' (from 'package:intl/intl.dart') hides import from 'dart:ui'.
Nit: line 1, column 235 of lib/gallery/item.dart: Import of 'Flow' (from 'package:flutter/src/widgets/basic.dart') hides import from 'dart:developer'.
47b9fb8b-f2b6-4cfc-87f7-339ba117b0fe flutter/flutter/examples/flutter_gallery/lib/main.dart.dill
aam commented 7 years ago

@peter-ahe-google @sigmundch

aam commented 7 years ago

@rmacnak-google

rmacnak-google commented 7 years ago

I wouldn't expect us to build a flow graph for a redirecting factory at all. Perhaps something neglected to follow the redirection.

aam commented 7 years ago

https://github.com/flutter/flutter/blob/master/examples/flutter_gallery/lib/demo/pesto_demo.dart#L75 is the redirecting constructor invocation, which is invoked from RecipeGridPage::createState at https://github.com/flutter/flutter/blob/master/examples/flutter_gallery/lib/demo/pesto_demo.dart#L71.

aam commented 7 years ago

@jensjoha

aam commented 7 years ago

Just remembered that let dynamic #redirecting_factory = fra2::LabeledGlobalKey::_ in invalid-expression; is a valid way to represent redirecting factory constructors in Kernel and the question is why GlobalKey constructor was invoked as it was supposed to be redirected to invocation of LabeledGlobalKey constructor.

aam commented 7 years ago

The issue seems to be with kernel file produced by incremental compilation. Instead of having invocation of target constructor(LabeledGlobalKey) produced in kernel file, it has invocation of redirecting constructor(GlobalKey):

full kernel file:

  class _RecipeGridPageState extends fra::State<pes::RecipeGridPage> {
    final field fra::GlobalKey<sca2::ScaffoldState> scaffoldKey = new fra::LabeledGlobalKey::_<sca2::ScaffoldState>();
    default constructor •() → void
      : super fra::State::•()
      ;
    @core::override

incremental kernel file:

class _RecipeGridPageState extends fra::State<pes::RecipeGridPage> {
    final field fra::GlobalKey<sca::ScaffoldState> scaffoldKey = fra::GlobalKey::•<sca::ScaffoldState>();
    default constructor •() → void
      : super fra::State::•()
      ;

@scheglov

scheglov commented 7 years ago

https://dart-review.googlesource.com/#/c/sdk/+/8381

a-siva commented 7 years ago

Can this be closed?

aam commented 7 years ago

Yes, all good now.