Compile and run Hello World!

Compile and run Hello World!

Committed: https://github.com/dart-lang/sdk/commit/1214e6fba636d76fca88357c358990c24b08c756

Reverted: https://github.com/dart-lang/sdk/commit/482502705ad05b6a14017f30b12909d97b322bdf
Committed: https://github.com/dart-lang/sdk/commit/99ff7d2b7794322f0379009e4feed595632106e4

[Johnni Winther, 2017-06-15 11:11:39]

johnniwinther@google.com changed reviewers:
+ efortuna@google.com, sigmund@google.com

[Emily Fortuna, 2017-06-15 17:54:34]

https://codereview.chromium.org/2942863002/diff/40001/pkg/compiler/lib/src/ke...
File pkg/compiler/lib/src/kernel/kernel_backend_strategy.dart (right):

https://codereview.chromium.org/2942863002/diff/40001/pkg/compiler/lib/src/ke...
pkg/compiler/lib/src/kernel/kernel_backend_strategy.dart:278: return null;
is this really better than throwing? I mean, it gets hello world compiling, but
it's hiding issues.

https://codereview.chromium.org/2942863002/diff/40001/pkg/compiler/lib/src/ss...
File pkg/compiler/lib/src/ssa/builder_kernel.dart (right):

https://codereview.chromium.org/2942863002/diff/40001/pkg/compiler/lib/src/ss...
pkg/compiler/lib/src/ssa/builder_kernel.dart:144: // No code is generated for
`targetElement`.
why?

https://codereview.chromium.org/2942863002/diff/40001/pkg/compiler/lib/src/ss...
File pkg/compiler/lib/src/ssa/ssa.dart (right):

https://codereview.chromium.org/2942863002/diff/40001/pkg/compiler/lib/src/ss...
pkg/compiler/lib/src/ssa/ssa.dart:75: /// return `true`.
return -> returns

https://codereview.chromium.org/2942863002/diff/40001/pkg/compiler/lib/src/ss...
pkg/compiler/lib/src/ssa/ssa.dart:84: // variables. For instance variables, we
may need to generate
sorry, I don't understand why we don't need to generate code for this situation?

[Siggi Cherem (dart-lang), 2017-06-15 18:28:55]

it's alive! and talking!

https://codereview.chromium.org/2942863002/diff/40001/pkg/compiler/lib/src/js...
File pkg/compiler/lib/src/js_backend/namer.dart (left):

https://codereview.chromium.org/2942863002/diff/40001/pkg/compiler/lib/src/js...
pkg/compiler/lib/src/js_backend/namer.dart:25: import '../tree/tree.dart';
\o/

https://codereview.chromium.org/2942863002/diff/40001/pkg/compiler/lib/src/ke...
File pkg/compiler/lib/src/kernel/kernel_backend_strategy.dart (right):

https://codereview.chromium.org/2942863002/diff/40001/pkg/compiler/lib/src/ke...
pkg/compiler/lib/src/kernel/kernel_backend_strategy.dart:278: return null;
On 2017/06/15 17:54:34, Emily Fortuna wrote:
> is this really better than throwing? I mean, it gets hello world compiling,
but
> it's hiding issues.

one option here: throw in checked mode only?

https://codereview.chromium.org/2942863002/diff/40001/pkg/compiler/lib/src/ss...
File pkg/compiler/lib/src/ssa/ssa.dart (right):

https://codereview.chromium.org/2942863002/diff/40001/pkg/compiler/lib/src/ss...
pkg/compiler/lib/src/ssa/ssa.dart:67: /// Handle field initializer of
`work.element`. Returns `true` if no code
nit: `work.element` => [element]
(here and below)

https://codereview.chromium.org/2942863002/diff/40001/tests/compiler/dart2js/...
File tests/compiler/dart2js/kernel/run_from_dill_test.dart (right):

https://codereview.chromium.org/2942863002/diff/40001/tests/compiler/dart2js/...
tests/compiler/dart2js/kernel/run_from_dill_test.dart:5: // Partial test that
the closed world computed from [WorldImpact]s derived from
fix doc

[Johnni Winther, 2017-06-16 09:24:09]

Description was changed from

==========
Compile and run Hello World!
==========

to

==========
Compile and run Hello World!

Committed:
https://github.com/dart-lang/sdk/commit/1214e6fba636d76fca88357c358990c24b08c756
==========

[Johnni Winther, 2017-06-16 09:24:10]

Committed patchset #4 (id:60001) manually as
1214e6fba636d76fca88357c358990c24b08c756 (presubmit successful).

[Johnni Winther, 2017-06-16 09:25:31]

No LGTM? - committed, anyway...

https://codereview.chromium.org/2942863002/diff/40001/pkg/compiler/lib/src/ke...
File pkg/compiler/lib/src/kernel/kernel_backend_strategy.dart (right):

https://codereview.chromium.org/2942863002/diff/40001/pkg/compiler/lib/src/ke...
pkg/compiler/lib/src/kernel/kernel_backend_strategy.dart:278: return null;
On 2017/06/15 18:28:55, Siggi Cherem (dart-lang) wrote:
> On 2017/06/15 17:54:34, Emily Fortuna wrote:
> > is this really better than throwing? I mean, it gets hello world compiling,
> but
> > it's hiding issues.
> 
> one option here: throw in checked mode only?

This is the price for the machete approach. We need to clean up the leaves
afterwards. In this case: returning `null` means [node] has no jumps to it,
which is true for loops without continue.

We run unittests in checked mode, so no.

https://codereview.chromium.org/2942863002/diff/40001/pkg/compiler/lib/src/ss...
File pkg/compiler/lib/src/ssa/builder_kernel.dart (right):

https://codereview.chromium.org/2942863002/diff/40001/pkg/compiler/lib/src/ss...
pkg/compiler/lib/src/ssa/builder_kernel.dart:144: // No code is generated for
`targetElement`.
On 2017/06/15 17:54:34, Emily Fortuna wrote:
> why?

All references inline the constant value. Add this to the comment.

https://codereview.chromium.org/2942863002/diff/40001/pkg/compiler/lib/src/ss...
File pkg/compiler/lib/src/ssa/ssa.dart (right):

https://codereview.chromium.org/2942863002/diff/40001/pkg/compiler/lib/src/ss...
pkg/compiler/lib/src/ssa/ssa.dart:67: /// Handle field initializer of
`work.element`. Returns `true` if no code
On 2017/06/15 18:28:55, Siggi Cherem (dart-lang) wrote:
> nit: `work.element` => [element]
> (here and below)

Done.

https://codereview.chromium.org/2942863002/diff/40001/pkg/compiler/lib/src/ss...
pkg/compiler/lib/src/ssa/ssa.dart:75: /// return `true`.
On 2017/06/15 17:54:34, Emily Fortuna wrote:
> return -> returns

Done.

https://codereview.chromium.org/2942863002/diff/40001/pkg/compiler/lib/src/ss...
pkg/compiler/lib/src/ssa/ssa.dart:84: // variables. For instance variables, we
may need to generate
On 2017/06/15 17:54:34, Emily Fortuna wrote:
> sorry, I don't understand why we don't need to generate code for this
situation?

All references inline the constant value. Add this to the comment.

https://codereview.chromium.org/2942863002/diff/40001/tests/compiler/dart2js/...
File tests/compiler/dart2js/kernel/run_from_dill_test.dart (right):

https://codereview.chromium.org/2942863002/diff/40001/tests/compiler/dart2js/...
tests/compiler/dart2js/kernel/run_from_dill_test.dart:5: // Partial test that
the closed world computed from [WorldImpact]s derived from
On 2017/06/15 18:28:55, Siggi Cherem (dart-lang) wrote:
> fix doc

Done.

[Johnni Winther, 2017-06-16 12:37:05]

Description was changed from

==========
Compile and run Hello World!

Committed:
https://github.com/dart-lang/sdk/commit/1214e6fba636d76fca88357c358990c24b08c756
==========

to

==========
Compile and run Hello World!

Committed:
https://github.com/dart-lang/sdk/commit/1214e6fba636d76fca88357c358990c24b08c756

Reverted:
https://github.com/dart-lang/sdk/commit/482502705ad05b6a14017f30b12909d97b322bdf
==========

[Johnni Winther, 2017-06-16 13:37:11]

Description was changed from

==========
Compile and run Hello World!

Committed:
https://github.com/dart-lang/sdk/commit/1214e6fba636d76fca88357c358990c24b08c756

Reverted:
https://github.com/dart-lang/sdk/commit/482502705ad05b6a14017f30b12909d97b322bdf
==========

to

==========
Compile and run Hello World!

Committed:
https://github.com/dart-lang/sdk/commit/1214e6fba636d76fca88357c358990c24b08c756

Reverted:
https://github.com/dart-lang/sdk/commit/482502705ad05b6a14017f30b12909d97b322bdf
Committed:
https://github.com/dart-lang/sdk/commit/99ff7d2b7794322f0379009e4feed595632106e4
==========

[Johnni Winther, 2017-06-16 13:37:13]

Committed patchset #5 (id:80001) manually as
99ff7d2b7794322f0379009e4feed595632106e4 (presubmit successful).

[Siggi Cherem (dart-lang), 2017-06-16 16:35:24]

no lgtm! because it was LGTAOU!! (all of us) ;-)

very exciting... very exciting

https://codereview.chromium.org/2942863002/diff/40001/pkg/compiler/lib/src/ke...
File pkg/compiler/lib/src/kernel/kernel_backend_strategy.dart (right):

https://codereview.chromium.org/2942863002/diff/40001/pkg/compiler/lib/src/ke...
pkg/compiler/lib/src/kernel/kernel_backend_strategy.dart:278: return null;
On 2017/06/16 09:25:31, Johnni Winther wrote:
> On 2017/06/15 18:28:55, Siggi Cherem (dart-lang) wrote:
> > On 2017/06/15 17:54:34, Emily Fortuna wrote:
> > > is this really better than throwing? I mean, it gets hello world
compiling,
> > but
> > > it's hiding issues.
> > 
> > one option here: throw in checked mode only?
> 
> This is the price for the machete approach. We need to clean up the leaves
> afterwards. In this case: returning `null` means [node] has no jumps to it,
> which is true for loops without continue.
> 
> We run unittests in checked mode, so no. 

How about:

if (const bool.fromEnvironment("MACHETE_CLEANUP")) throw "support jump targets";

[Emily Fortuna, 2017-06-16 17:38:14]

lgtm!!

[Johnni Winther, 2017-06-19 10:57:39]

https://codereview.chromium.org/2942863002/diff/40001/pkg/compiler/lib/src/ke...
File pkg/compiler/lib/src/kernel/kernel_backend_strategy.dart (right):

https://codereview.chromium.org/2942863002/diff/40001/pkg/compiler/lib/src/ke...
pkg/compiler/lib/src/kernel/kernel_backend_strategy.dart:278: return null;
On 2017/06/16 16:35:24, Siggi Cherem (dart-lang) wrote:
> On 2017/06/16 09:25:31, Johnni Winther wrote:
> > On 2017/06/15 18:28:55, Siggi Cherem (dart-lang) wrote:
> > > On 2017/06/15 17:54:34, Emily Fortuna wrote:
> > > > is this really better than throwing? I mean, it gets hello world
> compiling,
> > > but
> > > > it's hiding issues.
> > > 
> > > one option here: throw in checked mode only?
> > 
> > This is the price for the machete approach. We need to clean up the leaves
> > afterwards. In this case: returning `null` means [node] has no jumps to it,
> > which is true for loops without continue.
> > 
> > We run unittests in checked mode, so no. 
> 
> How about:
> 
> if (const bool.fromEnvironment("MACHETE_CLEANUP")) throw "support jump
targets";

I'll consider it.