Issue 10963055: [dart2dart] Make removal of asserts and forcing type strips go under

Issue 10963055: [dart2dart] Make removal of asserts and forcing type strips go under (Closed)

Created:
8 years, 3 months ago by Roman

Modified:
8 years, 2 months ago

Reviewers:
Anton Muhin, ahe

CC:
reviews_dartlang.org

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

Visibility:
Public.

More Reviews

Description

[dart2dart] Make removal of asserts and forcing type strips go under the flag --strip. e.g. --strip=types,asserts Committed: https://code.google.com/p/dart/source/detail?r=13175

Patch Set 3 : #

Created: 8 years, 2 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+44 lines, -23 lines)			Patch
M	lib/compiler/implementation/apiimpl.dart	View	1	1 chunk	+10 lines, -2 lines	0 comments	Download
M	lib/compiler/implementation/compiler.dart	View	1 2	2 chunks	+2 lines, -2 lines	0 comments	Download
M	lib/compiler/implementation/dart2js.dart	View	1 2	6 chunks	+13 lines, -3 lines	0 comments	Download
M	lib/compiler/implementation/dart_backend/backend.dart	View	1 2	7 chunks	+14 lines, -11 lines	0 comments	Download
M	tests/compiler/dart2js/dart_backend_test.dart	View	1 2	4 chunks	+5 lines, -5 lines	0 comments	Download

Messages

Total messages: 5 (0 generated)

Expand Messages | Collapse Messages

Anton Muhin

lgtm https://codereview.chromium.org/10963055/diff/1/lib/compiler/implementation/dart2js.dart File lib/compiler/implementation/dart2js.dart (right): https://codereview.chromium.org/10963055/diff/1/lib/compiler/implementation/dart2js.dart#newcode167 lib/compiler/implementation/dart2js.dart:167: if (outputLanguage != 'Dart' && stripArgumentSet) { maybe ...

8 years, 3 months ago (2012-09-24 13:09:19 UTC) #2

Roman

Peter, do you want to add anything? https://codereview.chromium.org/10963055/diff/1/lib/compiler/implementation/dart2js.dart File lib/compiler/implementation/dart2js.dart (right): https://codereview.chromium.org/10963055/diff/1/lib/compiler/implementation/dart2js.dart#newcode167 lib/compiler/implementation/dart2js.dart:167: if (outputLanguage ...

8 years, 2 months ago (2012-09-28 13:44:14 UTC) #3

ahe

LGTM https://chromiumcodereview.appspot.com/10963055/diff/6001/lib/compiler/implementation/apiimpl.dart File lib/compiler/implementation/apiimpl.dart (right): https://chromiumcodereview.appspot.com/10963055/diff/6001/lib/compiler/implementation/apiimpl.dart#newcode36 lib/compiler/implementation/apiimpl.dart:36: static List<String> getStrips(List<String> options) { I don't like ...

8 years, 2 months ago (2012-10-01 10:19:22 UTC) #4

Roman

8 years, 2 months ago (2012-10-03 12:24:26 UTC) #5

https://chromiumcodereview.appspot.com/10963055/diff/1/lib/compiler/implement...
File lib/compiler/implementation/dart2js.dart (right):

https://chromiumcodereview.appspot.com/10963055/diff/1/lib/compiler/implement...
lib/compiler/implementation/dart2js.dart:167: if (outputLanguage != 'Dart' &&
stripArgumentSet) {
On 2012/09/28 13:44:14, Roman wrote:
> On 2012/09/24 13:09:19, Anton Muhin wrote:
> > maybe it's time to introduced a named constant for outputLanguage?  Or maybe
> > even turn into enum-like thing.
> 
> Wouldn't enums be too heavy for such a small local thing? Although constants
> should be better that plain strings.

Added 'Dart' const.

https://chromiumcodereview.appspot.com/10963055/diff/6001/lib/compiler/implem...
File lib/compiler/implementation/apiimpl.dart (right):

https://chromiumcodereview.appspot.com/10963055/diff/6001/lib/compiler/implem...
lib/compiler/implementation/apiimpl.dart:36: static List<String>
getStrips(List<String> options) {
On 2012/10/01 10:19:22, ahe wrote:
> I don't like this method, but I don't like the hasOption method either. Long
> term, I think we should get rid of this. For example, by storing options in a
> map instead of a list.

I don't like it too. I agree, Map of args would certainly look better here.

Expand Messages | Collapse Messages