From 2e22f41612843a8791104803b6e906e5eb2ac358 Mon Sep 17 00:00:00 2001
From: Rico Wind <ricow@google.com>
Date: Mon, 14 Sep 2015 07:58:18 +0200
Subject: [PATCH] This is a fixed up version of:
 https://codereview.chromium.org/1305183010/

- Reorder the priority of flags in the test harness when passed to the VM:
  default flags, file flags, command line flags
- Ensure to not enable assertions when not being requested.

Peter, any reason to not move the flags in to the
configuration file?  Why don't we actually have all of them in there? (not
that I want to move them, I am just currious why we don't add them in
there, it seems more logical to me)

BUG=
R=iposva@google.com, whesse@google.com

Committed: https://github.com/dart-lang/sdk/commit/dd7767d95eda87054844e0b24ebd2ce9b04354bb

Review URL: https://codereview.chromium.org//1330643003 .
---
 runtime/vm/parser.cc                           |  2 +-
 tests/language/assertion_test.dart             |  2 +-
 tests/standalone/no_assert_test.dart           | 16 ++++++++++++++++
 tests/standalone/standalone.status             |  3 +++
 tools/testing/dart/compiler_configuration.dart | 11 ++++++++++-
 tools/testing/dart/test_suite.dart             |  4 ----
 6 files changed, 31 insertions(+), 7 deletions(-)
 create mode 100644 tests/standalone/no_assert_test.dart

diff --git a/runtime/vm/parser.cc b/runtime/vm/parser.cc
index d837830bcea..23c45fdd92e 100644
--- a/runtime/vm/parser.cc
+++ b/runtime/vm/parser.cc
@@ -9091,7 +9091,7 @@ AstNode* Parser::ParseAssertStatement() {
   ConsumeToken();  // Consume assert keyword.
   ExpectToken(Token::kLPAREN);
   const intptr_t condition_pos = TokenPos();
-  if (!I->flags().asserts() && !I->flags().type_checks()) {
+  if (!I->flags().asserts()) {
     SkipExpr();
     ExpectToken(Token::kRPAREN);
     return NULL;
diff --git a/tests/language/assertion_test.dart b/tests/language/assertion_test.dart
index 68bdf04b1b7..2f0a12d4962 100644
--- a/tests/language/assertion_test.dart
+++ b/tests/language/assertion_test.dart
@@ -1,7 +1,7 @@
 // Copyright (c) 2011, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
-// VMOptions=--enable_type_checks
+// VMOptions=--enable_asserts
 //
 // Dart test program testing assert statements.
 
diff --git a/tests/standalone/no_assert_test.dart b/tests/standalone/no_assert_test.dart
new file mode 100644
index 00000000000..825578b590d
--- /dev/null
+++ b/tests/standalone/no_assert_test.dart
@@ -0,0 +1,16 @@
+// Copyright (c) 2015, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+// VMOptions=--no-enable_asserts --enable_type_checks
+
+// Ensure that enabling of type checks does not automatically enable asserts.
+
+main() {
+  assert(false);
+  try {
+    int i = "String";
+    throw "FAIL";
+  } on TypeError catch (e) {
+    print("PASS");
+  }
+}
diff --git a/tests/standalone/standalone.status b/tests/standalone/standalone.status
index 03ca40dcbb4..4f1cd7031d7 100644
--- a/tests/standalone/standalone.status
+++ b/tests/standalone/standalone.status
@@ -17,6 +17,9 @@ issue14236_test: Pass # Do not remove this line. It serves as a marker for Issue
 
 javascript_compatibility_errors_test/none: Fail, OK  # Not possible to exclude or annotate with '/// none:'
 
+[ $runtime != vm ]
+no_assert_test: Fail, OK # This is testing a vm flag.
+
 [ $runtime == vm ]
 package/package_isolate_test: Fail # Issue 12474
 io/observatory_test: Fail
diff --git a/tools/testing/dart/compiler_configuration.dart b/tools/testing/dart/compiler_configuration.dart
index fc1d536e0e0..9ec90f00271 100644
--- a/tools/testing/dart/compiler_configuration.dart
+++ b/tools/testing/dart/compiler_configuration.dart
@@ -155,7 +155,12 @@ class NoneCompilerConfiguration extends CompilerConfiguration {
       List<String> sharedOptions,
       List<String> originalArguments,
       CommandArtifact artifact) {
-    return <String>[]
+    List<String> args = [];
+    if (isChecked) {
+      args.add('--enable_asserts');
+      args.add('--enable_type_checks');
+    }
+    return args
         ..addAll(vmOptions)
         ..addAll(sharedOptions)
         ..addAll(originalArguments);
@@ -315,6 +320,10 @@ class AnalyzerCompilerConfiguration extends CompilerConfiguration {
       CommandBuilder commandBuilder,
       List arguments,
       Map<String, String> environmentOverrides) {
+    arguments = new List.from(arguments);
+    if (isChecked) {
+      arguments.add('--enable_type_checks');
+    }
     return new CommandArtifact(
         <Command>[
             commandBuilder.getAnalysisCommand(
diff --git a/tools/testing/dart/test_suite.dart b/tools/testing/dart/test_suite.dart
index aef5b3f2a06..b4e7b537396 100644
--- a/tools/testing/dart/test_suite.dart
+++ b/tools/testing/dart/test_suite.dart
@@ -2225,10 +2225,6 @@ class TestUtils {
 
   static List<String> standardOptions(Map configuration) {
     List args = ["--ignore-unrecognized-flags"];
-    if (configuration["checked"]) {
-      args.add('--enable_asserts');
-      args.add("--enable_type_checks");
-    }
     String compiler = configuration["compiler"];
     if (compiler == "dart2js") {
       args = ['--generate-code-with-compile-time-errors', '--test-mode'];