Assertion failure in pkg/dev_compiler/test/expression_compiler/assertions_enabled_test

[johnniwinther]

The test pkg/dev_compiler/test/expression_compiler/assertions_enabled_test fails (somewhat) flakily after landing https://dart-review.googlesource.com/c/sdk/+/349864

The failure stack track is

  Expected: (contains 'Error: Assertion failed:' and contains '<unknown source>:-1:-1' and contains 'BoolLiteral(false)' and contains 'is not true')
    Actual: '[Internal error: Null check operator used on a null value:
              #0      ProgramCompiler.visitAssertStatement (package:dev_compiler/src/kernel/compiler.dart:4348:62)\n'
              '#1      AssertStatement.accept (package:kernel/ast.dart:9415:43)\n'
              '#2      ProgramCompiler._visitStatement (package:dev_compiler/src/kernel/compiler.dart:4116:20)\n'
              '#3      MappedListIterable.elementAt (dart:_internal/iterable.dart:425:31)\n'
              '#4      ListIterator.moveNext (dart:_internal/iterable.dart:354:26)\n'
              '#5      new _GrowableList._ofEfficientLengthIterable (dart:core-patch/growable_array.dart:189:27)\n'
              '#6      new _GrowableList.of (dart:core-patch/growable_array.dart:150:28)\n'
              '#7      new List.of (dart:core-patch/array_patch.dart:39:18)\n'
              '#8      ListIterable.toList (dart:_internal/iterable.dart:224:7)\n'
              '#9      ProgramCompiler.visitBlock (package:dev_compiler/src/kernel/compiler.dart:4297:62)\n'
              '#10     Block.accept (package:kernel/ast.dart:9271:43)\n'
              '#11     ProgramCompiler._visitStatement (package:dev_compiler/src/kernel/compiler.dart:4116:20)\n'
              '#12     ProgramCompiler._emitFunctionScopedBody (package:dev_compiler/src/kernel/compiler.dart:4149:18)\n'
              '#13     ProgramCompiler._emitSyncFunctionBody.<anonymous closure> (package:dev_compiler/src/kernel/compiler.dart:3855:17)\n'
              '#14     ProgramCompiler._withLetScope (package:dev_compiler/src/kernel/compiler.dart:2661:25)\n'
              '#15     ProgramCompiler._withCurrentFunction (package:dev_compiler/src/kernel/compiler.dart:3892:18)\n'
              '#16     ProgramCompiler._emitSyncFunctionBody (package:dev_compiler/src/kernel/compiler.dart:3851:17)\n'
              '#17     ProgramCompiler._emitFunction (package:dev_compiler/src/kernel/compiler.dart:3656:11)\n'
              '#18     ProgramCompiler._emitArrowFunction (package:dev_compiler/src/kernel/compiler.dart:7248:13)\n'
              '#19     ProgramCompiler.visitFunctionExpression (package:dev_compiler/src/kernel/compiler.dart:7241:14)\n'
              '#20     FunctionExpression.accept (package:kernel/ast.dart:8682:44)\n'
              '#21     ProgramCompiler._visitExpression (package:dev_compiler/src/kernel/compiler.dart:4207:20)\n'
              '#22     ProgramCompiler._emitMethodCall (package:dev_compiler/src/kernel/compiler.dart:5485:22)\n'
              '#23     ProgramCompiler.visitFunctionInvocation (package:dev_compiler/src/kernel/compiler.dart:5395:12)\n'
              '#24     FunctionInvocation.accept (package:kernel/ast.dart:6122:44)\n'
              '#25     ProgramCompiler._visitExpression (package:dev_compiler/src/kernel/compiler.dart:4207:20)\n'
              '#26     ProgramCompiler.visitReturnStatement (package:dev_compiler/src/kernel/compiler.dart:4894:45)\n'
              '#27     ReturnStatement.accept (package:kernel/ast.dart:10235:43)\n'
              '#28     ProgramCompiler._visitStatement (package:dev_compiler/src/kernel/compiler.dart:4116:20)\n'
              '#29     ProgramCompiler._emitFunctionScopedBody (package:dev_compiler/src/kernel/compiler.dart:4149:18)\n'
              '#30     ProgramCompiler._emitSyncFunctionBody.<anonymous closure> (package:dev_compiler/src/kernel/compiler.dart:3855:17)\n'
              '#31     ProgramCompiler._withLetScope (package:dev_compiler/src/kernel/compiler.dart:2661:25)\n'
              '#32     ProgramCompiler._withCurrentFunction (package:dev_compiler/src/kernel/compiler.dart:3892:18)\n'
              '#33     ProgramCompiler._emitSyncFunctionBody (package:dev_compiler/src/kernel/compiler.dart:3851:17)\n'
              '#34     ProgramCompiler._emitFunction (package:dev_compiler/src/kernel/compiler.dart:3656:11)\n'
              '#35     ProgramCompiler.emitFunctionIncremental (package:dev_compiler/src/kernel/compiler.dart:3597:15)\n'
              '#36     ExpressionCompiler._compileExpression (package:dev_compiler/src/kernel/expression_compiler.dart:240:36)\n'
              '<asynchronous suspension>\n'
              '#37     ExpressionCompiler.compileExpressionToJs (package:dev_compiler/src/kernel/expression_compiler.dart:122:26)\n'
              '<asynchronous suspension>\n'
              '#38     TestExpressionCompiler.compileExpression (file:///Volumes/Work/s/w/ir/cache/builder/sdk/pkg/dev_compiler/test/expression_compiler/test_compiler.dart:138:24)\n'
              '<asynchronous suspension>\n'
              '#39     ExpressionEvaluationTestDriver._compileDartExpressionInFrame (file:///Volumes/Work/s/w/ir/cache/builder/sdk/pkg/dev_compiler/test/expression_compiler/expression_compiler_e2e_suite.dart:682:12)\n'
              '<asynchronous suspension>\n'
              '#40     ExpressionEvaluationTestDriver._evaluateDartExpressionInFrame (file:///Volumes/Work/s/w/ir/cache/builder/sdk/pkg/dev_compiler/test/expression_compiler/expression_compiler_e2e_suite.dart:710:18)\n'
              '<asynchronous suspension>\n'
              '#41     ExpressionEvaluationTestDriver.checkInFrame.<anonymous closure> (file:///Volumes/Work/s/w/ir/cache/builder/sdk/pkg/dev_compiler/test/expression_compiler/expression_compiler_e2e_suite.dart:543:24)\n'
              '<asynchronous suspension>\n'
              '#42     ExpressionEvaluationTestDriver._onBreakpoint (file:///Volumes/Work/s/w/ir/cache/builder/sdk/pkg/dev_compiler/test/expression_compiler/expression_compiler_e2e_suite.dart:403:14)\n'
              '<asynchronous suspension>\n'
              '#43     ExpressionEvaluationTestDriver.checkInFrame (file:///Volumes/Work/s/w/ir/cache/builder/sdk/pkg/dev_compiler/test/expression_compiler/expression_compiler_e2e_suite.dart:542:12)\n'
              '<asynchronous suspension>\n'
              '#44     main.<anonymous closure>.<anonymous closure>.<anonymous closure> (file:///Volumes/Work/s/w/ir/cache/builder/sdk/pkg/dev_compiler/test/expression_compiler/assertions_enabled_test.dart:56:11)\n'
              '<asynchronous suspension>\n'
              '#45     Declarer.test.<anonymous closure>.<anonymous closure> (package:test_api/src/backend/declarer.dart:215:9)\n'
              '<asynchronous suspension>\n'
              '#46     Declarer.test.<anonymous closure> (package:test_api/src/backend/declarer.dart:213:7)\n'
              '<asynchronous suspension>\n'
              '#47     Invoker._waitForOutstandingCallbacks.<anonymous closure> (package:test_api/src/backend/invoker.dart:258:9)\n'
              '<asynchronous suspension>\n'
              ']'
     Which: does not contain 'Error: Assertion failed:'
  

  package:matcher/src/expect/expect.dart 149:31                                        fail
  package:matcher/src/expect/expect.dart 144:3                                         _expect
  package:matcher/src/expect/expect.dart 56:3                                          expect
  pkg/dev_compiler/test/expression_compiler/expression_compiler_e2e_suite.dart 555:9   ExpressionEvaluationTestDriver.checkInFrame.<fn>
  ===== asynchronous gap ===========================
  pkg/dev_compiler/test/expression_compiler/expression_compiler_e2e_suite.dart 403:14  ExpressionEvaluationTestDriver._onBreakpoint
  ===== asynchronous gap ===========================
  pkg/dev_compiler/test/expression_compiler/expression_compiler_e2e_suite.dart 542:12  ExpressionEvaluationTestDriver.checkInFrame
  ===== asynchronous gap ===========================
  pkg/dev_compiler/test/expression_compiler/assertions_enabled_test.dart 56:11         main.<fn>.<fn>.<fn>
  ===== asynchronous gap ===========================
  package:test_api/src/backend/declarer.dart 215:9                                     Declarer.test.<fn>.<fn>
  ===== asynchronous gap ===========================
  package:test_api/src/backend/declarer.dart 213:7                                     Declarer.test.<fn>
  ===== asynchronous gap ===========================
  package:test_api/src/backend/invoker.dart 258:9                                      Invoker._waitForOutstandingCallbacks.<fn>

[nshahan]

@johnniwinther This seems like it consistently fails for me. I suspect the tests were in a flaky state previously and the infra took a little while to recognize the consistent failure.

I did a little debugging and I can see that the source location for the assertion is different now during expression evaluation. The code being compiled that causes the failure is here:

sdk/pkg/dev_compiler/test/expression_compiler/assertions_enabled_test.dart

Line 58 in 8101536

expression: '() { assert(false); return 0; } ()',

Previously node.location was null and that was handled here:

sdk/pkg/dev_compiler/lib/src/kernel/compiler.dart

Lines 4078 to 4113 in 81cfa19

	if (node.location != null) {
	var encodedSource =
	node.enclosingComponent!.uriToSource[node.location!.file]!.source;
	var source = utf8.decode(encodedSource, allowMalformed: true);
	conditionSource =
	source.substring(node.conditionStartOffset, node.conditionEndOffset);
	location = _toSourceLocation(node.conditionStartOffset)!;
	} else {
	// Location is null in expression compilation when modules
	// are loaded from kernel using expression compiler worker.
	// Show the error only in that case, with the condition AST
	// instead of the source.
	//
	// TODO(annagrin): Can we add some information to the kernel,
	// or add better printing for the condition?
	// Issue: https://github.com/dart-lang/sdk/issues/43986
	conditionSource = node.condition.toString();
	}
	return js.statement(' if (!#) #;', [
	jsCondition,
	runtimeCall('assertFailed(#, #, #, #, #)', [
	if (node.message == null)
	js_ast.LiteralNull()
	else
	_visitExpression(node.message!),
	if (location == null)
	_cacheUri('<unknown source>')
	else
	_cacheUri(location.sourceUrl.toString()),
	// Lines and columns are typically printed with 1 based indexing.
	js.number(location == null ? -1 : location.line + 1),
	js.number(location == null ? -1 : location.column + 1),
	js.escapedString(conditionSource),
	])
	]);

The expecation in the test was that we would see "<unknown source>:-1:-1".

Now I see that node.location is "org-dartlang-debug:synthetic_debug_expression:1:6". This seems like a good change to me. Is this change expected and can I start relying on it going forward?

[johnniwinther]

We should always have offsets on asserts so we should rely on it. Don't have this changed, though.