dart-lang / sdk

The Dart SDK, including the VM, JS and Wasm compilers, analysis, core libraries, and more.
https://dart.dev
BSD 3-Clause "New" or "Revised" License
10.29k stars 1.59k forks source link

Help with clarification of tests intentions #29186

Open scheglov opened 7 years ago

scheglov commented 7 years ago

I landed a change that makes analyzer generate errors for the new top-level inference rules.

This caused some *_strong tests to start failing. I fixed most of them, but for some test I'm not sure whether I want to change them, or the existing code is required to test something very specific in DDC or VM.

corelib_strong/error_stack_trace2_test
language_strong/inferrer_constructor3_test.dart
lib_strong/html/input_element_test
scheglov commented 7 years ago

@munificent You landed at least one of these tests, but I don't know if you're the person who wrote them, so knows their intentions.

munificent commented 7 years ago

I think I just moved them into the new directories. I didn't write or modify the tests themselves.

It looks like language_strong/inferrer_constructor3_test is a dart2js regression test.

I would take a look at the history of the non-strong versions of each test and see who touched those last.

scheglov commented 7 years ago

@floitschG About corelib_strong/error_stack_trace2_test.

ERROR|COMPILE_TIME_ERROR|STRONG_MODE_TOP_LEVEL_INSTANCE_GETTER|/Users/scheglov/Source/Dart/sdk.git/sdk/tests/corelib_strong/error_stack_trace2_test.dart|12|27|5|The type of 'cyclicStatic' can't be inferred because of the use of the instance getter 'foo'.

Is it OK to break the type inference dependency cycle (and also not allowed instance getter reference) and declare cyclicStatic as int?

scheglov commented 7 years ago

@peter-ahe-google About language_strong/inferrer_constructor3_test.

ERROR|COMPILE_TIME_ERROR|STRONG_MODE_TOP_LEVEL_UNSUPPORTED|/Users/scheglov/Source/Dart/sdk.git/sdk/tests/language_strong/inferrer_constructor3_test.dart|17|15|15|The type of 'c' can't be inferred because IndexExpressionImpl expressions aren't supported.

Is it possible to rewrite this test so that it does not use index expression?

scheglov commented 7 years ago

@peter-ahe-google About lib_strong/html/input_element_test.

ERROR|COMPILE_TIME_ERROR|STRONG_MODE_TOP_LEVEL_UNSUPPORTED|/Users/scheglov/Source/Dart/sdk.git/sdk/tests/lib_strong/html/input_element_test.dart|192|25|14|The type of '_undefined' can't be inferred because IndexExpressionImpl expressions aren't supported.

Is it possible to rewrite this test so that it does not use index expression?

peter-ahe-google commented 7 years ago

@scheglov try changing the test to use dynamic instead of var.

scheglov commented 7 years ago

https://codereview.chromium.org/2806543002

munificent commented 7 years ago

Can this be closed now?