Make compile-time errors catchable

Make compile-time errors catchable

If an error happens during the compilation of a function body, an Error is thrown which can be intercepted, and ensures that finally blocks are executed before the isolate is terminated.

The language spec is vague about compilation errors. A doc describing the intentions behind this CL is at
https://docs.google.com/document/d/1_MWOgwJadLCQSBps0zD6Rj5dG4iP1UBuiDvTMH-WMzI/edit#

Example:
     1	void bad() {
     2	    return 5
     3	}
     4
     5	void main(args) {
     6	    bad();
     7	}

Before this CL:
$ dart ~/tmp/e.dart
'file:///Users/hausner/tmp/e.dart': error: line 2 pos 11: semicolon expected
  return 5
          ^
$

After this change:
$ dart ~/tmp/e.dart
Unhandled exception:
'file:///Users/hausner/tmp/e.dart': error: line 2 pos 11: semicolon expected
  return 5
          ^

#0      main (file:///Users/hausner/tmp/e.dart:6:3)
#1      _startIsolate.<anonymous closure> (dart:isolate-patch/isolate_patch.dart:259)
#2      _RawReceivePortImpl._handleMessage (dart:isolate-patch/isolate_patch.dart:148)
$

Notice that the stack trace points to the call site of bad(), not the text location of the syntax error. That's not a bug. The location of the syntax error is given in the error message.


BUG= https://github.com/dart-lang/sdk/issues/23684
R=asiva@google.com, lrn@google.com

Committed: https://github.com/dart-lang/sdk/commit/3e0d13bc287cc89ebc15969376a90bd7f67bef6a

[kevmoo, 2016-07-19 21:37:08]

Description was changed from

==========
Experimental change to make syntax errors catchable

May want to add text position and url properties to
SyntaxError class.

Many co19 tests fail since they contain a catch-all clause but
expect to see a compile-time error.

Dart2js needs to be changed accordingly so the platforms match.

The language spec is vague about compilation errors, but need to
check whether it needs to be updated.


BUG=23684
==========

to

==========
Experimental change to make syntax errors catchable

May want to add text position and url properties to
SyntaxError class.

Many co19 tests fail since they contain a catch-all clause but
expect to see a compile-time error.

Dart2js needs to be changed accordingly so the platforms match.

The language spec is vague about compilation errors, but need to
check whether it needs to be updated.


BUG= https://github.com/dart-lang/sdk/issues/23684
==========

[Lasse Reichstein Nielsen, 2016-07-22 11:46:03]

> Dart2js needs to be changed accordingly so the platforms match.

I don't think dart2js needs to change. Reporting all syntax errors up-front is
always allowed - it's "compile-time" errors after all.

It's only the VM that interleaves compile-time with run-time, and compile-time
errors happening after code has run that needs to be handled.

[hausner, 2016-09-14 21:53:30]

Description was changed from

==========
Experimental change to make syntax errors catchable

May want to add text position and url properties to
SyntaxError class.

Many co19 tests fail since they contain a catch-all clause but
expect to see a compile-time error.

Dart2js needs to be changed accordingly so the platforms match.

The language spec is vague about compilation errors, but need to
check whether it needs to be updated.


BUG= https://github.com/dart-lang/sdk/issues/23684
==========

to

==========
Experimental change to make syntax errors catchable

May want to add text position and url properties to
SyntaxError class. For now they are just contained in the
error text.

Dart2js should be changed accordingly so the platforms match.

The language spec is vague about compilation errors. A doc describing the
intentions behind this CL is at
https://docs.google.com/document/d/1_MWOgwJadLCQSBps0zD6Rj5dG4iP1UBuiDvTMH-WM...


BUG= https://github.com/dart-lang/sdk/issues/23684
==========

[hausner, 2016-09-14 22:29:41]

hausner@google.com changed reviewers:
+ asiva@google.com

[hausner, 2016-09-14 22:52:35]

Description was changed from

==========
Experimental change to make syntax errors catchable

May want to add text position and url properties to
SyntaxError class. For now they are just contained in the
error text.

Dart2js should be changed accordingly so the platforms match.

The language spec is vague about compilation errors. A doc describing the
intentions behind this CL is at
https://docs.google.com/document/d/1_MWOgwJadLCQSBps0zD6Rj5dG4iP1UBuiDvTMH-WM...


BUG= https://github.com/dart-lang/sdk/issues/23684
==========

to

==========
Experimental change to make syntax errors catchable

May want to add text position and url properties to
SyntaxError class. For now they are just contained in the
error text.

Dart2js should be changed accordingly so the platforms match.

The language spec is vague about compilation errors. A doc describing the
intentions behind this CL is at
https://docs.google.com/document/d/1_MWOgwJadLCQSBps0zD6Rj5dG4iP1UBuiDvTMH-WM...

Example:
     1	void bad() {
     2	    return 5
     3	}
     4	
     5	void main(args) {
     6	    bad();
     7	}

Before this CL:
$ dart ~/tmp/e.dart
'file:///Users/hausner/tmp/e.dart': error: line 2 pos 11: semicolon expected
  return 5
          ^
$

After this change:
$ dart ~/tmp/e.dart
Unhandled exception:
'file:///Users/hausner/tmp/e.dart': error: line 2 pos 11: semicolon expected
  return 5
          ^

#0      main (file:///Users/hausner/tmp/e.dart:6:3)
#1      _startIsolate.<anonymous closure>
(dart:isolate-patch/isolate_patch.dart:259)
#2      _RawReceivePortImpl._handleMessage
(dart:isolate-patch/isolate_patch.dart:148)
$ 

BUG= https://github.com/dart-lang/sdk/issues/23684
==========

[hausner, 2016-09-14 22:54:20]

Description was changed from

==========
Experimental change to make syntax errors catchable

May want to add text position and url properties to
SyntaxError class. For now they are just contained in the
error text.

Dart2js should be changed accordingly so the platforms match.

The language spec is vague about compilation errors. A doc describing the
intentions behind this CL is at
https://docs.google.com/document/d/1_MWOgwJadLCQSBps0zD6Rj5dG4iP1UBuiDvTMH-WM...

Example:
     1	void bad() {
     2	    return 5
     3	}
     4	
     5	void main(args) {
     6	    bad();
     7	}

Before this CL:
$ dart ~/tmp/e.dart
'file:///Users/hausner/tmp/e.dart': error: line 2 pos 11: semicolon expected
  return 5
          ^
$

After this change:
$ dart ~/tmp/e.dart
Unhandled exception:
'file:///Users/hausner/tmp/e.dart': error: line 2 pos 11: semicolon expected
  return 5
          ^

#0      main (file:///Users/hausner/tmp/e.dart:6:3)
#1      _startIsolate.<anonymous closure>
(dart:isolate-patch/isolate_patch.dart:259)
#2      _RawReceivePortImpl._handleMessage
(dart:isolate-patch/isolate_patch.dart:148)
$ 

BUG= https://github.com/dart-lang/sdk/issues/23684
==========

to

==========
Experimental change to make syntax errors catchable

May want to add text position and url properties to
SyntaxError class. For now they are just contained in the
error text.

Dart2js should be changed accordingly so the platforms match.

The language spec is vague about compilation errors. A doc describing the
intentions behind this CL is at
https://docs.google.com/document/d/1_MWOgwJadLCQSBps0zD6Rj5dG4iP1UBuiDvTMH-WM...

Example:
     1	void bad() {
     2	    return 5
     3	}
     4	
     5	void main(args) {
     6	    bad();
     7	}

Before this CL:
$ dart ~/tmp/e.dart
'file:///Users/hausner/tmp/e.dart': error: line 2 pos 11: semicolon expected
  return 5
          ^
$

After this change:
$ dart ~/tmp/e.dart
Unhandled exception:
'file:///Users/hausner/tmp/e.dart': error: line 2 pos 11: semicolon expected
  return 5
          ^

#0      main (file:///Users/hausner/tmp/e.dart:6:3)
#1      _startIsolate.<anonymous closure>
(dart:isolate-patch/isolate_patch.dart:259)
#2      _RawReceivePortImpl._handleMessage
(dart:isolate-patch/isolate_patch.dart:148)
$ 

Notice that the stack trace points to the call site of bad(), not the text
location of the syntax error. That's not a bug. The location of the syntax error
is given in the error message.


BUG= https://github.com/dart-lang/sdk/issues/23684
==========

[Lasse Reichstein Nielsen, 2016-09-15 05:41:34]

lrn@google.com changed reviewers:
+ lrn@google.com

[Lasse Reichstein Nielsen, 2016-09-15 05:41:35]

https://codereview.chromium.org/2044753002/diff/20001/sdk/lib/core/errors.dart
File sdk/lib/core/errors.dart (right):

https://codereview.chromium.org/2044753002/diff/20001/sdk/lib/core/errors.dar...
sdk/lib/core/errors.dart:576: class SyntaxError extends Error {
I have two objections here:

1. This will represent compile-time errors, not all of which are syntax errors.
A better name would be `CompileTimeError`.

2. I'm not sure I want to expose this class. The dart2js code won't need it for
anything, and its current implementation isn't adding anything important for the
user - it's just wrapping a message.
I hope no user will want to catch a CompileTimeError directly. If you catch all
errors, it makes sense to catch CompileTimeError too, but singling out
CompileTimeErrors with an `on CompileTimeError catch (e)` is probably just being
too clever.
Making it a VM-private class that still extends Error is sufficient for me, and
I'd like to see if it's sufficient for other users as well.

The subclasses of Error that we have are there because they carry important
information for debugging and for creating a good error string in a consistent
way, and not really so you can identify them by class (not that people don't but
it's not the intent). This class does neither: It doesn't expose important
information as data, and it isn't intended for users to create instances of to
help them have consistent error messages, so it has no reason to be user
visible.

[siva, 2016-09-15 18:11:45]

maybe change the title from 'Experimental change' to 'Make syntax errors
catchable'

I tend to agree with Lasse that the SyntaxError class could be made a VM class
similar to _InternalError.

LGTM otherwise.

https://codereview.chromium.org/2044753002/diff/20001/runtime/vm/dart_api_imp...
File runtime/vm/dart_api_impl.cc (right):

https://codereview.chromium.org/2044753002/diff/20001/runtime/vm/dart_api_imp...
runtime/vm/dart_api_impl.cc:190:
Class::Handle(I->object_store()->syntax_error_class());
Handle(zone, I->object_store()....
as the zone seems to be passed in as a parameter.

https://codereview.chromium.org/2044753002/diff/20001/runtime/vm/exceptions.cc
File runtime/vm/exceptions.cc (right):

https://codereview.chromium.org/2044753002/diff/20001/runtime/vm/exceptions.c...
runtime/vm/exceptions.cc:726: class_name = &Symbols::SyntaxError();
Does this need to be
  break;

default:
  UNREACHABLE();

like the way we do in other switch statements?

[hausner, 2016-09-15 21:22:13]

Description was changed from

==========
Experimental change to make syntax errors catchable

May want to add text position and url properties to
SyntaxError class. For now they are just contained in the
error text.

Dart2js should be changed accordingly so the platforms match.

The language spec is vague about compilation errors. A doc describing the
intentions behind this CL is at
https://docs.google.com/document/d/1_MWOgwJadLCQSBps0zD6Rj5dG4iP1UBuiDvTMH-WM...

Example:
     1	void bad() {
     2	    return 5
     3	}
     4	
     5	void main(args) {
     6	    bad();
     7	}

Before this CL:
$ dart ~/tmp/e.dart
'file:///Users/hausner/tmp/e.dart': error: line 2 pos 11: semicolon expected
  return 5
          ^
$

After this change:
$ dart ~/tmp/e.dart
Unhandled exception:
'file:///Users/hausner/tmp/e.dart': error: line 2 pos 11: semicolon expected
  return 5
          ^

#0      main (file:///Users/hausner/tmp/e.dart:6:3)
#1      _startIsolate.<anonymous closure>
(dart:isolate-patch/isolate_patch.dart:259)
#2      _RawReceivePortImpl._handleMessage
(dart:isolate-patch/isolate_patch.dart:148)
$ 

Notice that the stack trace points to the call site of bad(), not the text
location of the syntax error. That's not a bug. The location of the syntax error
is given in the error message.


BUG= https://github.com/dart-lang/sdk/issues/23684
==========

to

==========
Make compile-time errors catchable

If an error happens during the compilation of a function body, an Error is
thrown which can be intercepted, and ensures that finally blocks are executed
before the isolate is terminated.

The language spec is vague about compilation errors. A doc describing the
intentions behind this CL is at
https://docs.google.com/document/d/1_MWOgwJadLCQSBps0zD6Rj5dG4iP1UBuiDvTMH-WM...

Example:
     1	void bad() {
     2	    return 5
     3	}
     4	
     5	void main(args) {
     6	    bad();
     7	}

Before this CL:
$ dart ~/tmp/e.dart
'file:///Users/hausner/tmp/e.dart': error: line 2 pos 11: semicolon expected
  return 5
          ^
$

After this change:
$ dart ~/tmp/e.dart
Unhandled exception:
'file:///Users/hausner/tmp/e.dart': error: line 2 pos 11: semicolon expected
  return 5
          ^

#0      main (file:///Users/hausner/tmp/e.dart:6:3)
#1      _startIsolate.<anonymous closure>
(dart:isolate-patch/isolate_patch.dart:259)
#2      _RawReceivePortImpl._handleMessage
(dart:isolate-patch/isolate_patch.dart:148)
$ 

Notice that the stack trace points to the call site of bad(), not the text
location of the syntax error. That's not a bug. The location of the syntax error
is given in the error message.


BUG= https://github.com/dart-lang/sdk/issues/23684
==========

[hausner, 2016-09-15 21:28:29]

PTAL. The error class is now vm-only and private.

https://codereview.chromium.org/2044753002/diff/20001/runtime/vm/dart_api_imp...
File runtime/vm/dart_api_impl.cc (right):

https://codereview.chromium.org/2044753002/diff/20001/runtime/vm/dart_api_imp...
runtime/vm/dart_api_impl.cc:190:
Class::Handle(I->object_store()->syntax_error_class());
On 2016/09/15 18:11:45, siva wrote:
> Handle(zone, I->object_store()....
> as the zone seems to be passed in as a parameter.

Done.

https://codereview.chromium.org/2044753002/diff/20001/runtime/vm/exceptions.cc
File runtime/vm/exceptions.cc (right):

https://codereview.chromium.org/2044753002/diff/20001/runtime/vm/exceptions.c...
runtime/vm/exceptions.cc:726: class_name = &Symbols::SyntaxError();
On 2016/09/15 18:11:45, siva wrote:
> Does this need to be
>   break;
> 
> default:
>   UNREACHABLE();
> 
> like the way we do in other switch statements?

Done.

There does not seem to be a default case, though.

https://codereview.chromium.org/2044753002/diff/20001/sdk/lib/core/errors.dart
File sdk/lib/core/errors.dart (right):

https://codereview.chromium.org/2044753002/diff/20001/sdk/lib/core/errors.dar...
sdk/lib/core/errors.dart:576: class SyntaxError extends Error {
On 2016/09/15 05:41:35, Lasse Reichstein Nielsen wrote:
> I have two objections here:
> 
> 1. This will represent compile-time errors, not all of which are syntax
errors.
> A better name would be `CompileTimeError`.
> 
> 2. I'm not sure I want to expose this class. The dart2js code won't need it
for
> anything, and its current implementation isn't adding anything important for
the
> user - it's just wrapping a message.
> I hope no user will want to catch a CompileTimeError directly. If you catch
all
> errors, it makes sense to catch CompileTimeError too, but singling out
> CompileTimeErrors with an `on CompileTimeError catch (e)` is probably just
being
> too clever.
> Making it a VM-private class that still extends Error is sufficient for me,
and
> I'd like to see if it's sufficient for other users as well.
> 
> The subclasses of Error that we have are there because they carry important
> information for debugging and for creating a good error string in a consistent
> way, and not really so you can identify them by class (not that people don't
but
> it's not the intent). This class does neither: It doesn't expose important
> information as data, and it isn't intended for users to create instances of to
> help them have consistent error messages, so it has no reason to be user
> visible.

Renamed SyntaxError to _CompileTimeError. The class is VM-only and not
user-visible.

[siva, 2016-09-16 00:12:09]

still LGTM.

https://codereview.chromium.org/2044753002/diff/80001/runtime/vm/dart_api_imp...
File runtime/vm/dart_api_impl.cc (right):

https://codereview.chromium.org/2044753002/diff/80001/runtime/vm/dart_api_imp...
runtime/vm/dart_api_impl.cc:195: return false;
maybe

return (obj.GetClassId() == error_class.id());

[Lasse Reichstein Nielsen, 2016-09-16 05:34:29]

lgtm

https://codereview.chromium.org/2044753002/diff/80001/runtime/lib/errors_patc...
File runtime/lib/errors_patch.dart (right):

https://codereview.chromium.org/2044753002/diff/80001/runtime/lib/errors_patc...
runtime/lib/errors_patch.dart:366: String toString() => _errorMsg;
If the error messages are self explanatory, this is fine. I guess they are since
it was the same message you saw before.
Otherwise you might want to have "Compilation error: $_errorMsg".

[hausner, 2016-09-16 17:10:43]

Description was changed from

==========
Make compile-time errors catchable

If an error happens during the compilation of a function body, an Error is
thrown which can be intercepted, and ensures that finally blocks are executed
before the isolate is terminated.

The language spec is vague about compilation errors. A doc describing the
intentions behind this CL is at
https://docs.google.com/document/d/1_MWOgwJadLCQSBps0zD6Rj5dG4iP1UBuiDvTMH-WM...

Example:
     1	void bad() {
     2	    return 5
     3	}
     4	
     5	void main(args) {
     6	    bad();
     7	}

Before this CL:
$ dart ~/tmp/e.dart
'file:///Users/hausner/tmp/e.dart': error: line 2 pos 11: semicolon expected
  return 5
          ^
$

After this change:
$ dart ~/tmp/e.dart
Unhandled exception:
'file:///Users/hausner/tmp/e.dart': error: line 2 pos 11: semicolon expected
  return 5
          ^

#0      main (file:///Users/hausner/tmp/e.dart:6:3)
#1      _startIsolate.<anonymous closure>
(dart:isolate-patch/isolate_patch.dart:259)
#2      _RawReceivePortImpl._handleMessage
(dart:isolate-patch/isolate_patch.dart:148)
$ 

Notice that the stack trace points to the call site of bad(), not the text
location of the syntax error. That's not a bug. The location of the syntax error
is given in the error message.


BUG= https://github.com/dart-lang/sdk/issues/23684
==========

to

==========
Make compile-time errors catchable

If an error happens during the compilation of a function body, an Error is
thrown which can be intercepted, and ensures that finally blocks are executed
before the isolate is terminated.

The language spec is vague about compilation errors. A doc describing the
intentions behind this CL is at
https://docs.google.com/document/d/1_MWOgwJadLCQSBps0zD6Rj5dG4iP1UBuiDvTMH-WM...

Example:
     1	void bad() {
     2	    return 5
     3	}
     4
     5	void main(args) {
     6	    bad();
     7	}

Before this CL:
$ dart ~/tmp/e.dart
'file:///Users/hausner/tmp/e.dart': error: line 2 pos 11: semicolon expected
  return 5
          ^
$

After this change:
$ dart ~/tmp/e.dart
Unhandled exception:
'file:///Users/hausner/tmp/e.dart': error: line 2 pos 11: semicolon expected
  return 5
          ^

#0      main (file:///Users/hausner/tmp/e.dart:6:3)
#1      _startIsolate.<anonymous closure>
(dart:isolate-patch/isolate_patch.dart:259)
#2      _RawReceivePortImpl._handleMessage
(dart:isolate-patch/isolate_patch.dart:148)
$

Notice that the stack trace points to the call site of bad(), not the text
location of the syntax error. That's not a bug. The location of the syntax error
is given in the error message.


BUG= https://github.com/dart-lang/sdk/issues/23684
R=asiva@google.com, lrn@google.com

Committed:
https://github.com/dart-lang/sdk/commit/3e0d13bc287cc89ebc15969376a90bd7f67bef6a
==========

[hausner, 2016-09-16 17:10:44]

Committed patchset #6 (id:100001) manually as
3e0d13bc287cc89ebc15969376a90bd7f67bef6a (presubmit successful).