Skip to content

Commit

Permalink
Make compile-time errors catchable
Browse files Browse the repository at this point in the history
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= #23684
[email protected], [email protected]

Review URL: https://codereview.chromium.org/2044753002 .
  • Loading branch information
mhausner committed Sep 16, 2016
1 parent b9a5c77 commit 3e0d13b
Show file tree
Hide file tree
Showing 13 changed files with 62 additions and 5 deletions.
7 changes: 7 additions & 0 deletions runtime/lib/errors_patch.dart
Original file line number Diff line number Diff line change
Expand Up @@ -358,3 +358,10 @@ class _InternalError {
return msg_buf.toString();
}
}


class _CompileTimeError extends Error {
final String _errorMsg;
_CompileTimeError(this._errorMsg);
String toString() => _errorMsg;
}
1 change: 1 addition & 0 deletions runtime/vm/clustered_snapshot.cc
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ class ClassSerializationCluster : public SerializationCluster {
s->WriteRef(*p);
}
intptr_t class_id = cls->ptr()->id_;
ASSERT(class_id != kIllegalCid);
s->WriteCid(class_id);
s->Write<int32_t>(cls->ptr()->instance_size_in_words_);
s->Write<int32_t>(cls->ptr()->next_field_offset_in_words_);
Expand Down
4 changes: 4 additions & 0 deletions runtime/vm/compiler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,10 @@ DEFINE_RUNTIME_ENTRY(CompileFunction, 1) {
const Error& error =
Error::Handle(Compiler::CompileFunction(thread, function));
if (!error.IsNull()) {
if (error.IsLanguageError()) {
Exceptions::ThrowCompileTimeError(LanguageError::Cast(error));
UNREACHABLE();
}
Exceptions::PropagateError(error);
}
}
Expand Down
16 changes: 16 additions & 0 deletions runtime/vm/dart_api_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,15 @@ static RawInstance* GetMapInstance(Zone* zone, const Object& obj) {
}


static bool IsCompiletimeErrorObject(Zone* zone, const Object& obj) {
Isolate* I = Thread::Current()->isolate();
const Class& error_class =
Class::Handle(zone, I->object_store()->compiletime_error_class());
ASSERT(!error_class.IsNull());
return (obj.GetClassId() == error_class.id());
}


static bool GetNativeStringArgument(NativeArguments* arguments,
int arg_index,
Dart_Handle* str,
Expand Down Expand Up @@ -761,6 +770,13 @@ DART_EXPORT bool Dart_IsUnhandledExceptionError(Dart_Handle object) {


DART_EXPORT bool Dart_IsCompilationError(Dart_Handle object) {
if (Dart_IsUnhandledExceptionError(object)) {
DARTSCOPE(Thread::Current());
const UnhandledException& error =
UnhandledException::Cast(Object::Handle(Z, Api::UnwrapHandle(object)));
const Instance& exc = Instance::Handle(Z, error.exception());
return IsCompiletimeErrorObject(Z, exc);
}
return Api::ClassId(object) == kLanguageErrorCid;
}

Expand Down
3 changes: 0 additions & 3 deletions runtime/vm/dart_api_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -570,7 +570,6 @@ TEST_CASE(Dart_PropagateError) {

result = Dart_Invoke(lib, NewString("Func1"), 0, NULL);
EXPECT(Dart_IsError(result));
EXPECT(!Dart_ErrorHasException(result));
EXPECT_SUBSTRING("semicolon expected", Dart_GetError(result));

result = Dart_Invoke(lib, NewString("Func2"), 0, NULL);
Expand All @@ -584,7 +583,6 @@ TEST_CASE(Dart_PropagateError) {

result = Dart_Invoke(lib, NewString("Func1"), 0, NULL);
EXPECT(Dart_IsError(result));
EXPECT(!Dart_ErrorHasException(result));
EXPECT_SUBSTRING("semicolon expected", Dart_GetError(result));

result = Dart_Invoke(lib, NewString("Func2"), 0, NULL);
Expand All @@ -598,7 +596,6 @@ TEST_CASE(Dart_PropagateError) {

result = Dart_Invoke(lib, NewString("Func1"), 0, NULL);
EXPECT(Dart_IsError(result));
EXPECT(!Dart_ErrorHasException(result));
EXPECT_SUBSTRING("semicolon expected", Dart_GetError(result));

result = Dart_Invoke(lib, NewString("Func2"), 0, NULL);
Expand Down
12 changes: 12 additions & 0 deletions runtime/vm/exceptions.cc
Original file line number Diff line number Diff line change
Expand Up @@ -651,6 +651,13 @@ void Exceptions::ThrowRangeError(const char* argument_name,
}


void Exceptions::ThrowCompileTimeError(const LanguageError& error) {
const Array& args = Array::Handle(Array::New(1));
args.SetAt(0, String::Handle(error.FormatMessage()));
Exceptions::ThrowByType(Exceptions::kCompileTimeError, args);
}


RawObject* Exceptions::Create(ExceptionType type, const Array& arguments) {
Library& library = Library::Handle();
const String* class_name = NULL;
Expand Down Expand Up @@ -724,6 +731,11 @@ RawObject* Exceptions::Create(ExceptionType type, const Array& arguments) {
case kCyclicInitializationError:
library = Library::CoreLibrary();
class_name = &Symbols::CyclicInitializationError();
break;
case kCompileTimeError:
library = Library::CoreLibrary();
class_name = &Symbols::_CompileTimeError();
break;
}

return DartLibraryCalls::InstanceCreate(library,
Expand Down
3 changes: 3 additions & 0 deletions runtime/vm/exceptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ class AbstractType;
class Array;
class DartFrameIterator;
class Error;
class LanguageError;
class Instance;
class Integer;
class RawInstance;
Expand Down Expand Up @@ -60,6 +61,7 @@ class Exceptions : AllStatic {
kFallThrough,
kAbstractClassInstantiation,
kCyclicInitializationError,
kCompileTimeError,
};

static void ThrowByType(ExceptionType type, const Array& arguments);
Expand All @@ -72,6 +74,7 @@ class Exceptions : AllStatic {
const Integer& argument_value,
intptr_t expected_from,
intptr_t expected_to);
static void ThrowCompileTimeError(const LanguageError& error);

// Returns a RawInstance if the exception is successfully created,
// otherwise returns a RawError.
Expand Down
3 changes: 2 additions & 1 deletion runtime/vm/object.h
Original file line number Diff line number Diff line change
Expand Up @@ -5349,6 +5349,8 @@ class LanguageError : public Error {

virtual const char* ToErrorCString() const;

TokenPosition token_pos() const { return raw_ptr()->token_pos_; }

private:
RawError* previous_error() const {
return raw_ptr()->previous_error_;
Expand All @@ -5358,7 +5360,6 @@ class LanguageError : public Error {
RawScript* script() const { return raw_ptr()->script_; }
void set_script(const Script& value) const;

TokenPosition token_pos() const { return raw_ptr()->token_pos_; }
void set_token_pos(TokenPosition value) const;

bool report_after_token() const { return raw_ptr()->report_after_token_; }
Expand Down
6 changes: 6 additions & 0 deletions runtime/vm/object_store.cc
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ ObjectStore::ObjectStore()
int32x4_type_(Type::null()),
float64x2_type_(Type::null()),
string_type_(Type::null()),
compiletime_error_class_(Class::null()),
future_class_(Class::null()),
completer_class_(Class::null()),
stream_iterator_class_(Class::null()),
Expand Down Expand Up @@ -243,6 +244,11 @@ void ObjectStore::InitKnownObjects() {
cls = internal_lib.LookupClass(Symbols::Symbol());
set_symbol_class(cls);

const Library& core_lib = Library::Handle(core_library());
cls = core_lib.LookupClassAllowPrivate(Symbols::_CompileTimeError());
ASSERT(!cls.IsNull());
set_compiletime_error_class(cls);

// Cache the core private functions used for fast instance of checks.
simple_instance_of_function_ =
PrivateObjectLookup(Symbols::_simpleInstanceOf());
Expand Down
8 changes: 8 additions & 0 deletions runtime/vm/object_store.h
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,13 @@ class ObjectStore {
string_type_ = value.raw();
}

RawClass* compiletime_error_class() const {
return compiletime_error_class_;
}
void set_compiletime_error_class(const Class& value) {
compiletime_error_class_ = value.raw();
}

RawClass* future_class() const { return future_class_; }
void set_future_class(const Class& value) {
future_class_ = value.raw();
Expand Down Expand Up @@ -532,6 +539,7 @@ class ObjectStore {
V(RawType*, int32x4_type_) \
V(RawType*, float64x2_type_) \
V(RawType*, string_type_) \
V(RawClass*, compiletime_error_class_) \
V(RawClass*, future_class_) \
V(RawClass*, completer_class_) \
V(RawClass*, stream_iterator_class_) \
Expand Down
1 change: 1 addition & 0 deletions runtime/vm/precompiler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -389,6 +389,7 @@ void Precompiler::DoCompileAll(
I->object_store()->set_completer_class(null_class);
I->object_store()->set_stream_iterator_class(null_class);
I->object_store()->set_symbol_class(null_class);
I->object_store()->set_compiletime_error_class(null_class);
}
DropClasses();
DropLibraries();
Expand Down
2 changes: 1 addition & 1 deletion runtime/vm/raw_object.h
Original file line number Diff line number Diff line change
Expand Up @@ -1603,7 +1603,7 @@ class RawLanguageError : public RawError {
}
TokenPosition token_pos_; // Source position in script_.
bool report_after_token_; // Report message at or after the token.
int8_t kind_; // Of type LanguageError::Kind.
int8_t kind_; // Of type Report::Kind.
};


Expand Down
1 change: 1 addition & 0 deletions runtime/vm/symbols.h
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ class ObjectPointerVisitor;
V(AbstractClassInstantiationError, "AbstractClassInstantiationError") \
V(NoSuchMethodError, "NoSuchMethodError") \
V(CyclicInitializationError, "CyclicInitializationError") \
V(_CompileTimeError, "_CompileTimeError") \
V(ThrowNew, "_throwNew") \
V(ThrowNewIfNotLoaded, "_throwNewIfNotLoaded") \
V(CheckAssertion, "_checkAssertion") \
Expand Down

0 comments on commit 3e0d13b

Please sign in to comment.