Issue 10836188: Proper support for VMOptions declarations for dart2dart.

Issue 10836188: Proper support for VMOptions declarations for dart2dart. (Closed)

Created:
8 years, 4 months ago by Anton Muhin

Modified:
8 years, 4 months ago

Reviewers:
Bill Hesse

CC:
reviews_dartlang.org, Roman

Base URL:
https://dart.googlecode.com/svn/branches/bleeding_edge/dart

Visibility:
Public.

More Reviews

Description

Proper support for VMOptions declarations for dart2dart. Plus remove suppressions for now passing tests and retriage ones for which we hoped the reason was in VMOptions. Committed: https://code.google.com/p/dart/source/detail?r=10563

Patch Set 1 #

Patch Set 2 : #

Patch Set 3 : #

Patch Set 4 : #

Total comments: 6

Patch Set 5 : #

Created: 8 years, 4 months ago

Download [raw] [tar.bz2]

		Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+38 lines, -50 lines)			Patch
	M	tests/language/language.status	View	1 2 3 4	7 chunks	+6 lines, -23 lines	0 comments	Download
	M	tools/testing/dart/test_suite.dart	View	1 2 3 4	6 chunks	+32 lines, -27 lines	0 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

Bill Hesse

LGTM, with comments. https://chromiumcodereview.appspot.com/10836188/diff/5001/tools/testing/dart/test_suite.dart File tools/testing/dart/test_suite.dart (right): https://chromiumcodereview.appspot.com/10836188/diff/5001/tools/testing/dart/test_suite.dart#newcode439 tools/testing/dart/test_suite.dart:439: var arguments = new List.from(vmOptions); We ...

8 years, 4 months ago (2012-08-13 08:13:11 UTC) #2

Anton Muhin

8 years, 4 months ago (2012-08-13 09:23:34 UTC) #3

https://chromiumcodereview.appspot.com/10836188/diff/5001/tools/testing/dart/...
File tools/testing/dart/test_suite.dart (right):

https://chromiumcodereview.appspot.com/10836188/diff/5001/tools/testing/dart/...
tools/testing/dart/test_suite.dart:439: var arguments = new
List.from(vmOptions);
On 2012/08/13 08:13:11, Bill Hesse wrote:
> We do not need to add vmOptions to dart2js.  They are zeroed out in this case
> much earlier.  If we eliminate this here, then we can eliminate the zeroing
out.
> 
> In fact, we can eliminate the entire function getVmOptions at line 1035. 
Let's
> do that now.

Done.

https://chromiumcodereview.appspot.com/10836188/diff/5001/tools/testing/dart/...
tools/testing/dart/test_suite.dart:459: var vmArguments = new
List.from(vmOptions);
Sorry, I do not quite follow.

On 2012/08/13 08:13:11, Bill Hesse wrote:
> Let's add runtime == 'vm' before adding vmOptions, in both places.  In that
> case, you don't need to split the default case into none and dartc, but I
think
> it would still be good.

https://chromiumcodereview.appspot.com/10836188/diff/5001/tools/testing/dart/...
tools/testing/dart/test_suite.dart:470: default:
On 2012/08/13 08:13:11, Bill Hesse wrote:
> Perhaps split the default case into 'none' and 'dartc', and add vmoptions only
> to none.  'frog' should be eliminated.
> 

Done.

Expand Messages | Collapse Messages