Report malformed URIs in library dependencies

Report malformed URIs in library dependencies as errors.

For instance: "import '../../Udyn[mic ils/expect.dart';" used to cause an internal compiler error. The error is now caught and the compiler reports the invalid import.

BUG=http://dartbug.com/23228
R=floitsch@google.com

Committed: https://code.google.com/p/dart/source/detail?r=45580

[Harry Terkelsen, 2015-05-06 21:09:08]

het@google.com changed reviewers:
+ floitsch@google.com

[floitsch, 2015-05-06 21:11:22]

Please add a better subject and description.

[Harry Terkelsen, 2015-05-06 21:15:17]

On 2015/05/06 21:11:22, floitsch wrote:
> Please add a better subject and description.

Done

[floitsch, 2015-05-06 21:19:01]

LGTM.

(Please use http://dartbug.com/XYZ as bugnumber, as codereview otherwise points
to the chromium bug).

https://codereview.chromium.org/1131593006/diff/20001/pkg/compiler/lib/src/li...
File pkg/compiler/lib/src/library_loader.dart (right):

https://codereview.chromium.org/1131593006/diff/20001/pkg/compiler/lib/src/li...
pkg/compiler/lib/src/library_loader.dart:515: return new Future.value();
This return feels fishy.
Is the convention that this function returns a Future<null> if it couldn't load
the library? If yes, this should be part of the comment.

My guess is, that reportError was fatal and something changed.
Alternative guess is that the test's error-collector is not fatal (whereas the
dart2js one is).

In either case it's worth adding more documentation:
- add it as comment in the dartdoc.
- add a comment here: // compiler.reportError might not abort computation.

[Harry Terkelsen, 2015-05-07 00:05:55]

On 2015/05/06 21:19:01, floitsch wrote:
> LGTM.
> 
> (Please use http://dartbug.com/XYZ as bugnumber, as codereview otherwise
points
> to the chromium bug).
> 
>
https://codereview.chromium.org/1131593006/diff/20001/pkg/compiler/lib/src/li...
> File pkg/compiler/lib/src/library_loader.dart (right):
> 
>
https://codereview.chromium.org/1131593006/diff/20001/pkg/compiler/lib/src/li...
> pkg/compiler/lib/src/library_loader.dart:515: return new Future.value();
> This return feels fishy.
> Is the convention that this function returns a Future<null> if it couldn't
load
> the library? If yes, this should be part of the comment.
This function always returns Future<Null>. I added some comments and made the
type more specific.

> My guess is, that reportError was fatal and something changed.
> Alternative guess is that the test's error-collector is not fatal (whereas the
> dart2js one is).
> 
> In either case it's worth adding more documentation:
> - add it as comment in the dartdoc.
> - add a comment here: // compiler.reportError might not abort computation.
Added the comments. reportError does not immediately abort compilation unless a
flag is set

[Harry Terkelsen, 2015-05-07 00:11:55]

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