Issue 250: Allow for GNU formatted errors

Issue 250: Allow for GNU formatted errors

Removes --machine-problems option and added --error_format [normal|machine|gnu]

R=zundel@google.com
BUG=
TEST=

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

[ahe, 2011-12-21 22:39:46]

-zundle
+zundel

[ahe, 2011-12-21 23:11:13]

I don't understand why you keep the format foo.dart/bar.dart. As far as I can
tell it is nothing but trouble.

Also, this adds more complexity to the compiler than I'm comfortable with. It
shifts the complexity on the user.

Here are examples of when "foo.dart/bar.dart" convention causes problems:

* Parsing error messages from Emacs

* Parsing error messages from Xcode (I guess)

* Attempting to select the name in gnome-terminal (Linux) or Terminal.app (Mac).

* Humans trying to understand the error message.

Notice that these problems gets even worse when you get an absolute URI in
there:

file:/Users/ahe/Dart/all/dart.googlecode.com/dart/frog/demo/fisk.dart/src/fisk.dart:1:
Unexpected token '!' (expected 'IDENTIFIER')
     1: BAH!

If I really try hard, I can imagine that the "foo.dart/bar.dart" convention is
slightly better than nothing if I have included the same source file in multiple
libraries. However, that is an *uncommon* situation and you make everyone having
to deal with it (in a way that is incompatible with everything else).

The following alternatives exist:

1. Really simple, ignore the problem. Just print the URI.

2. Simple and useful, do like gcc:

In file included from fisk.c:1:
fisk.h:1:2: error: #error 

3. Complicated and probably overkill, identify when a file has been included
from multiple libraries, and only then say where it was included from (using
alternative 2).

4. Lame-o VM solution, see issue 180.

My vote is for alternative 2 and no additional compiler options.

[codefu, 2011-12-21 23:22:23]

Use "--error_format gnu".  The other error formats are in use by other
programmers for debugging, testing, and library creation.  The entire
"library/file" uri was to make each inclusion of a source file in different
libraries unique:

--foo.dart--
foofunc() {
  print("${GLOBAL_IN_A}");
}

--a.dart--
#library("a");
#import("b.dart");
#source("foo.dart");
int GLOBAL_IN_A = 42;
main() {
  Foo.foofunc();
}

--b.dart--
#library("b");
#import("c.dart",prefix: "c");
xmain() {
  c.Foo.foofunc();
}

--c.dart--
#library("b");
#import("c.dart",prefix: "c");
xmain() {
  c.Foo.foofunc();
}

-- compile --
dartc a.dart
c.dart/foo.dart:4: cannot resolve GLOBAL_IN_A
     3:   static foofunc() {
     4:     print("${GLOBAL_IN_A}");
Compilation failed with 1 problem.

Here you can clearly see the source of the problem, instead of "foo.dart" all
alone and with no context.

On 2011/12/21 23:11:13, ahe wrote:
> I don't understand why you keep the format foo.dart/bar.dart. As far as I can
> tell it is nothing but trouble.
> 
> Also, this adds more complexity to the compiler than I'm comfortable with. It
> shifts the complexity on the user.
> 
> Here are examples of when "foo.dart/bar.dart" convention causes problems:
> 
> * Parsing error messages from Emacs
> 
> * Parsing error messages from Xcode (I guess)
> 
> * Attempting to select the name in gnome-terminal (Linux) or Terminal.app
(Mac).
> 
> * Humans trying to understand the error message.
> 
> Notice that these problems gets even worse when you get an absolute URI in
> there:
> 
>
file:/Users/ahe/Dart/all/dart.googlecode.com/dart/frog/demo/fisk.dart/src/fisk.dart:1:
> Unexpected token '!' (expected 'IDENTIFIER')
>      1: BAH!
> 
> If I really try hard, I can imagine that the "foo.dart/bar.dart" convention is
> slightly better than nothing if I have included the same source file in
multiple
> libraries. However, that is an *uncommon* situation and you make everyone
having
> to deal with it (in a way that is incompatible with everything else).
> 
> The following alternatives exist:
> 
> 1. Really simple, ignore the problem. Just print the URI.
> 
> 2. Simple and useful, do like gcc:
> 
> In file included from fisk.c:1:
> fisk.h:1:2: error: #error 
> 
> 3. Complicated and probably overkill, identify when a file has been included
> from multiple libraries, and only then say where it was included from (using
> alternative 2).
> 
> 4. Lame-o VM solution, see issue 180.
> 
> My vote is for alternative 2 and no additional compiler options.

[ahe, 2011-12-21 23:26:20]

On 2011/12/21 23:22:23, codefu wrote:
> c.dart/foo.dart:4: cannot resolve GLOBAL_IN_A
>      3:   static foofunc() {
>      4:     print("${GLOBAL_IN_A}");
> Compilation failed with 1 problem.
> 
> Here you can clearly see the source of the problem, instead of "foo.dart" all
> alone and with no context.

No you cannot. That is the problem. "c.dart/foo.dart" is not clear. It looks
like you're reporting an error in the file foo.dart in the directory c.dart.

[Brian Wilkerson, 2011-12-22 00:17:24]

In one of the issues I have requested an API that would get the "unmunged"
version of the URI (having only the munged form available causes trouble for the
editor as well). If that were available then you could print it out, even if
only for this option.

[codefu, 2011-12-22 00:40:25]

On 2011/12/21 23:26:20, ahe wrote:
> On 2011/12/21 23:22:23, codefu wrote:
> > c.dart/foo.dart:4: cannot resolve GLOBAL_IN_A
> >      3:   static foofunc() {
> >      4:     print("${GLOBAL_IN_A}");
> > Compilation failed with 1 problem.
> > 
> > Here you can clearly see the source of the problem, instead of "foo.dart"
all
> > alone and with no context.
> 
> No you cannot. That is the problem. "c.dart/foo.dart" is not clear. It looks
> like you're reporting an error in the file foo.dart in the directory c.dart.

Changing the fact that libraries are also dart files is a different feature
request.  Changing the fact that you have to include files as "xyx.dart" is also
a different feature request. Does the "--error_format gnu" fix your problem or
not?  I am not interested in remove two features that other developers are
using, nor am I interested in your interpretation of "helpful" as I've heard
"library.dart/source.dart" is more helpful from others.

[ahe, 2011-12-22 00:52:09]

On 2011/12/22 00:40:25, codefu wrote:
> On 2011/12/21 23:26:20, ahe wrote:
> > On 2011/12/21 23:22:23, codefu wrote:
> > > c.dart/foo.dart:4: cannot resolve GLOBAL_IN_A
> > >      3:   static foofunc() {
> > >      4:     print("${GLOBAL_IN_A}");
> > > Compilation failed with 1 problem.
> > > 
> > > Here you can clearly see the source of the problem, instead of "foo.dart"
> all
> > > alone and with no context.
> > 
> > No you cannot. That is the problem. "c.dart/foo.dart" is not clear. It looks
> > like you're reporting an error in the file foo.dart in the directory c.dart.
> 
> Changing the fact that libraries are also dart files is a different feature
> request.

You lost me there.

>  Changing the fact that you have to include files as "xyx.dart" is also
> a different feature request. Does the "--error_format gnu" fix your problem or
> not?

It is not solving the problem. You cannot separate two file-paths with a slash
(/). This is because a slash is part of a file-path (on Windows, Linux, and Mac)
and URIs.

When you report an error message that includes "c.dart/foo.dart" it is
ambiguous.

> I am not interested in remove two features that other developers are
> using, nor am I interested in your interpretation of "helpful" as I've heard
> "library.dart/source.dart" is more helpful from others.

I have suggested four different alternatives. One of them is to remove the
feature. The three others aren't, they are suggestions to improve the existing
feature.

[codefu, 2011-12-22 00:58:52]

On 2011/12/22 00:52:09, ahe wrote:
> On 2011/12/22 00:40:25, codefu wrote:
> > Changing the fact that libraries are also dart files is a different feature
> > request.
> 
> You lost me there.
"library.dart" in the name is the source library file.  There's no
"library.lib".  I previously asked if library:file would work better for you,
but I believe you didn't like that one.

> >  Changing the fact that you have to include files as "xyx.dart" is also
> > a different feature request. Does the "--error_format gnu" fix your problem
or
> > not?
> 
> It is not solving the problem. You cannot separate two file-paths with a slash
> (/). This is because a slash is part of a file-path (on Windows, Linux, and
Mac)
> and URIs.

In issue 250, you suggested:
-            sourceFile.getName(),
+            sourceFile.getUri(),
Passing the gnu flag incorporates that change request.  I can see that I missed
DefaultErrorFormatter.java, which if your whole complaint was "you missed a
file", then I would understand.

[ahe, 2011-12-22 01:01:54]

On 2011/12/22 00:58:52, codefu wrote:
> On 2011/12/22 00:52:09, ahe wrote:
> > On 2011/12/22 00:40:25, codefu wrote:
> > > Changing the fact that libraries are also dart files is a different
feature
> > > request.
> > 
> > You lost me there.
> "library.dart" in the name is the source library file.  There's no
> "library.lib".  I previously asked if library:file would work better for you,
> but I believe you didn't like that one.
> 
> > >  Changing the fact that you have to include files as "xyx.dart" is also
> > > a different feature request. Does the "--error_format gnu" fix your
problem
> or
> > > not?
> > 
> > It is not solving the problem. You cannot separate two file-paths with a
slash
> > (/). This is because a slash is part of a file-path (on Windows, Linux, and
> Mac)
> > and URIs.
> 
> In issue 250, you suggested:
> -            sourceFile.getName(),
> +            sourceFile.getUri(),
> Passing the gnu flag incorporates that change request.  I can see that I
missed
> DefaultErrorFormatter.java, which if your whole complaint was "you missed a
> file", then I would understand.

Do what ever you like. I give up. I truly don't understand how to communicate
with you.

[zundel, 2011-12-22 13:32:42]

On 2011/12/22 01:01:54, ahe wrote:
> On 2011/12/22 00:58:52, codefu wrote:
> > On 2011/12/22 00:52:09, ahe wrote:
> > > On 2011/12/22 00:40:25, codefu wrote:
> > > > Changing the fact that libraries are also dart files is a different
> feature
> > > > request.
> > > 
> > > You lost me there.
> > "library.dart" in the name is the source library file.  There's no
> > "library.lib".  I previously asked if library:file would work better for
you,
> > but I believe you didn't like that one.
> > 
> > > >  Changing the fact that you have to include files as "xyx.dart" is also
> > > > a different feature request. Does the "--error_format gnu" fix your
> problem
> > or
> > > > not?
> > > 
> > > It is not solving the problem. You cannot separate two file-paths with a
> slash
> > > (/). This is because a slash is part of a file-path (on Windows, Linux,
and
> > Mac)
> > > and URIs.
> > 
> > In issue 250, you suggested:
> > -            sourceFile.getName(),
> > +            sourceFile.getUri(),
> > Passing the gnu flag incorporates that change request.  I can see that I
> missed
> > DefaultErrorFormatter.java, which if your whole complaint was "you missed a
> > file", then I would understand.
> 
> Do what ever you like. I give up. I truly don't understand how to communicate
> with you.

Hi Peter,

Our goal in this change was to make error messages work for one of our valued
users in emacs, which would be you.  The feedback we are looking for was whether
this change helps you out in that regard or not.

[codefu, 2011-12-22 15:25:09]

After talking to Eric, the previous example I listed was a copy/paste error.
Here's the source:

-- a.dart --
#library("a");
#import("b.dart");
#source("foo.dart");
int GLOBAL_VARIABLE = 42;
xy
main() {
  Foo.foofunc();
  xmain();
}

-- b.dart --
#library("b");
#import("c.dart", prefix: "c");
xmain() {
  c.Foo.foofunc();
}

-- c.dart --
#library('c');
#source("foo.dart");

-- foo.dart --
// I'm not a library, but let me depend on a global
class Foo {
  static foofunc() {
    print("${GLOBAL_VARIABLE}");
  }
}

And here's the new output; which should make all parties happy:

dartc gnu/a.dart
gnu/a.dart:5: no such type "xy"
     4: int GLOBAL_VARIABLE = 42;
     5: xy
gnu/foo.dart:4: cannot resolve GLOBAL_VARIABLE (sourced from library gnu/c.dart)
     3:   static foofunc() {
     4:     print("${GLOBAL_VARIABLE}");

[zundel, 2011-12-22 15:32:31]

lgtm

http://codereview.chromium.org/8949055/diff/9001/compiler/java/com/google/dar...
File compiler/java/com/google/dart/compiler/CommandLineOptions.java (right):

http://codereview.chromium.org/8949055/diff/9001/compiler/java/com/google/dar...
compiler/java/com/google/dart/compiler/CommandLineOptions.java:46: usage =
"Format errors as normal, machine, or GNU")
remove gnu

http://codereview.chromium.org/8949055/diff/9001/compiler/java/com/google/dar...
File compiler/java/com/google/dart/compiler/DefaultErrorFormatter.java (right):

http://codereview.chromium.org/8949055/diff/9001/compiler/java/com/google/dar...
compiler/java/com/google/dart/compiler/DefaultErrorFormatter.java:46:
includeFrom = " (sourced from library " + lib.getUri() + ")";
for the sake of brevity, could we drop the word 'library' and shorten to
'sourced from'?  I doubt most users think of their top level .dart file as a
library, even though that's technically true.

[codefu, 2011-12-22 15:38:20]

http://codereview.chromium.org/8949055/diff/1/compiler/java/com/google/dart/c...
File compiler/java/com/google/dart/compiler/ast/viz/BaseASTWriter.java (right):

http://codereview.chromium.org/8949055/diff/1/compiler/java/com/google/dart/c...
compiler/java/com/google/dart/compiler/ast/viz/BaseASTWriter.java:537:
write("DartSyntheticErrorExpression(", node, node.getTokenString() + ")");
This slipped in by accident.

http://codereview.chromium.org/8949055/diff/9001/compiler/java/com/google/dar...
File compiler/java/com/google/dart/compiler/CommandLineOptions.java (right):

http://codereview.chromium.org/8949055/diff/9001/compiler/java/com/google/dar...
compiler/java/com/google/dart/compiler/CommandLineOptions.java:46: usage =
"Format errors as normal, machine, or GNU")
On 2011/12/22 15:32:31, zundel wrote:
> remove gnu

Done.

http://codereview.chromium.org/8949055/diff/9001/compiler/java/com/google/dar...
File compiler/java/com/google/dart/compiler/DefaultErrorFormatter.java (right):

http://codereview.chromium.org/8949055/diff/9001/compiler/java/com/google/dar...
compiler/java/com/google/dart/compiler/DefaultErrorFormatter.java:46:
includeFrom = " (sourced from library " + lib.getUri() + ")";
On 2011/12/22 15:32:31, zundel wrote:
> for the sake of brevity, could we drop the word 'library' and shorten to
> 'sourced from'?  I doubt most users think of their top level .dart file as a
> library, even though that's technically true. 

Sure thing.

[ahe, 2011-12-22 15:38:33]

http://codereview.chromium.org/8949055/diff/9001/compiler/java/com/google/dar...
File compiler/java/com/google/dart/compiler/DefaultErrorFormatter.java (right):

http://codereview.chromium.org/8949055/diff/9001/compiler/java/com/google/dar...
compiler/java/com/google/dart/compiler/DefaultErrorFormatter.java:46:
includeFrom = " (sourced from library " + lib.getUri() + ")";
On 2011/12/22 15:32:31, zundel wrote:
> for the sake of brevity, could we drop the word 'library' and shorten to
> 'sourced from'?  I doubt most users think of their top level .dart file as a
> library, even though that's technically true. 

FWIW, I agree.