Cleanup of error and warning reporting.

Cleanup of error and warning reporting.

R=hausner@google.com, srdjan@google.com

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

[srdjan, 2014-06-18 21:26:43]

lgtm

https://codereview.chromium.org/340203003/diff/1/runtime/vm/class_finalizer.h
File runtime/vm/class_finalizer.h (right):

https://codereview.chromium.org/340203003/diff/1/runtime/vm/class_finalizer.h...
runtime/vm/class_finalizer.h:174: static void ReportErrors(const Error& error,
For documentation purpose: s/error/prev_error/

https://codereview.chromium.org/340203003/diff/1/runtime/vm/parser.h
File runtime/vm/parser.h (right):

https://codereview.chromium.org/340203003/diff/1/runtime/vm/parser.h#newcode317
runtime/vm/parser.h:317: // Concatenate and report and already formatted error
and a new error message.
s/and/an/

[hausner, 2014-06-18 21:48:53]

LGTM w/comments. Not sure whether the renaming of Parser::ErrorMsg was
necessary, but you did the work already...

https://codereview.chromium.org/340203003/diff/1/runtime/lib/identical.cc
File runtime/lib/identical.cc (right):

https://codereview.chromium.org/340203003/diff/1/runtime/lib/identical.cc#new...
runtime/lib/identical.cc:18: const bool is_static_native = true;  //
Identical_comparison is static.
ditto

https://codereview.chromium.org/340203003/diff/1/runtime/lib/object.cc
File runtime/lib/object.cc (right):

https://codereview.chromium.org/340203003/diff/1/runtime/lib/object.cc#newcod...
runtime/lib/object.cc:134: Report::JSWarningFromNative(is_static_native,
As discussed offline, I prefer passing false as a literal and a comment next to
it if necessary. Naming the constant is confusing, esp. for false values. I read
is_static_native = false and conclude that something is NOT static and native.

https://codereview.chromium.org/340203003/diff/1/runtime/vm/parser.cc
File runtime/vm/parser.cc (left):

https://codereview.chromium.org/340203003/diff/1/runtime/vm/parser.cc#oldcode...
runtime/vm/parser.cc:7541: void Parser::Warning(intptr_t token_pos, const char*
format, ...) {
Please don't get rid of these. Can you keep them as convenience function that
call Report::MessageF(Report::kWarning, ...  ?

https://codereview.chromium.org/340203003/diff/1/runtime/vm/report.h
File runtime/vm/report.h (right):

https://codereview.chromium.org/340203003/diff/1/runtime/vm/report.h#newcode34
runtime/vm/report.h:34: // Concatenate and report and already formatted error
and a new error message.
an already formatted ...

[regis, 2014-06-18 22:13:28]

Thanks!

https://codereview.chromium.org/340203003/diff/1/runtime/lib/identical.cc
File runtime/lib/identical.cc (right):

https://codereview.chromium.org/340203003/diff/1/runtime/lib/identical.cc#new...
runtime/lib/identical.cc:18: const bool is_static_native = true;  //
Identical_comparison is static.
On 2014/06/18 21:48:53, hausner wrote:
> ditto

Done.

https://codereview.chromium.org/340203003/diff/1/runtime/lib/object.cc
File runtime/lib/object.cc (right):

https://codereview.chromium.org/340203003/diff/1/runtime/lib/object.cc#newcod...
runtime/lib/object.cc:134: Report::JSWarningFromNative(is_static_native,
On 2014/06/18 21:48:53, hausner wrote:
> As discussed offline, I prefer passing false as a literal and a comment next
to
> it if necessary. Naming the constant is confusing, esp. for false values. I
read
> is_static_native = false and conclude that something is NOT static and native.

Done.

https://codereview.chromium.org/340203003/diff/1/runtime/vm/class_finalizer.h
File runtime/vm/class_finalizer.h (right):

https://codereview.chromium.org/340203003/diff/1/runtime/vm/class_finalizer.h...
runtime/vm/class_finalizer.h:174: static void ReportErrors(const Error& error,
On 2014/06/18 21:26:42, srdjan wrote:
> For documentation purpose: s/error/prev_error/ 

Done here, in parser, and in report.

https://codereview.chromium.org/340203003/diff/1/runtime/vm/parser.cc
File runtime/vm/parser.cc (left):

https://codereview.chromium.org/340203003/diff/1/runtime/vm/parser.cc#oldcode...
runtime/vm/parser.cc:7541: void Parser::Warning(intptr_t token_pos, const char*
format, ...) {
On 2014/06/18 21:48:53, hausner wrote:
> Please don't get rid of these. Can you keep them as convenience function that
> call Report::MessageF(Report::kWarning, ...  ?

Done.

https://codereview.chromium.org/340203003/diff/1/runtime/vm/parser.h
File runtime/vm/parser.h (right):

https://codereview.chromium.org/340203003/diff/1/runtime/vm/parser.h#newcode317
runtime/vm/parser.h:317: // Concatenate and report and already formatted error
and a new error message.
On 2014/06/18 21:26:42, srdjan wrote:
> s/and/an/

Done.

https://codereview.chromium.org/340203003/diff/1/runtime/vm/report.h
File runtime/vm/report.h (right):

https://codereview.chromium.org/340203003/diff/1/runtime/vm/report.h#newcode34
runtime/vm/report.h:34: // Concatenate and report and already formatted error
and a new error message.
On 2014/06/18 21:48:53, hausner wrote:
> an already formatted ...

Done.

[regis, 2014-06-18 22:30:45]

Committed patchset #3 manually as r37468 (presubmit successful).