Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

CFE: report compile-time errors for invalid spread collections in constants #36286

Closed
alexmarkov opened this issue Mar 20, 2019 · 2 comments
Closed

CFE: report compile-time errors for invalid spread collections in constants #36286

alexmarkov opened this issue Mar 20, 2019 · 2 comments
Assignees
@kmillikin
Labels
area-front-end Use area-front-end for front end / CFE / kernel format related issues.
Milestone
Dart 2.3

Comments

@alexmarkov
Copy link
Contributor

Certain test cases in language_2/spread_collections/const_error_test pass for wrong reason:

tools/test.py --print-passing-stdout -n dartk-linux-debug-x64 language_2/spread_collections/const_error_test/16

tools/testing/dart/main.dart:1: Warning: Interpreting this as package URI, 'package:test_dart/main.dart'.
Test configuration:
    dartk-linux-debug-x64(architecture: x64, compiler: dartk, mode: debug, runtime: vm, system: linux, vm-options: [], dart2js-options: [], timeout: null, preview-dart-2)
Suites tested: language_2

exit code:
254

stderr:
Unhandled exception:
'file:///[...]/out/DebugX64/generated_tests/language_2/spread_collections/const_error_test_16.dart': error: Not a constant expression: unexpected kernel tag InvalidExpression (19)
#0      main ([...]/out/DebugX64/generated_tests/language_2/spread_collections/const_error_test_16.dart:20:3)
#1      _startIsolate.<anonymous closure> (dart:isolate/runtime/lib/isolate_patch.dart:300:19)
#2      _RawReceivePortImpl._handleMessage (dart:isolate/runtime/lib/isolate_patch.dart:171:12)
[00:01 | 100% | +    1 | -    0]

I think this should be caught in CFE.

This is also causing problems in bytecode generator as error is not reported, but constant evaluator fails to evaluate a constant.

/cc @kmillikin @askeksa-google
@alexmarkov alexmarkov added the area-front-end Use area-front-end for front end / CFE / kernel format related issues. label Mar 20, 2019
@alexmarkov alexmarkov added this to the Dart2.3 milestone Mar 20, 2019
@kmillikin
Copy link

Constant evaluation errors will be caught in the CFE by the Fasta's constant evaluator, which is under the experimental feature flag constant-update-2018. Note that we will have to enable that flag for UI as code tests.

Once we switch that on, there will only be one constant evaluator and the bytecode generator should work fine.

dart-bot pushed a commit that referenced this issue Mar 26, 2019
@alexmarkov
[vm/bytecode] Declare members in bytecode
61f0f5b 
This change replaces kernel AST declarations of fields and functions
with bytecode declarations.

Size of dilp files is reduced by 11-12%.

Startup latency:
Time to the first full frame: 1.945s -> 1.687s
FinalizeClass: 554ms -> 277ms
FinishClassLoading: 296ms -> 156ms

There are following regressions in bytecode mode, which will be fixed
in future:

* dart:mirrors are not supported yet (implementation of mirrors relies
  on reading kernel AST in certain cases).

  As the result, lib_2/mirrors/* tests fail.

* native extensions are not supported yet (annotations on libraries
  and classes in AST are cleaned up as they could reference members
  which are now removed from AST).

  As the result, standalone_2/entrypoints_verification_test test fails.

* language_2/spread_collections/const_error_test/* tests fail
  due to #36286.

Change-Id: I5130f401fd7b84038b136136e7ccc1a6e51b6cea
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/97561
Commit-Queue: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
@kmillikin
Copy link

As far as I can tell, this is fixed by turning on CFE constant evaluation. The error is now reported by Fasta:

out/ReleaseX64/generated_tests/language_2/spread_collections/const_error_test_16.dart:67:9: Context: While analyzing:
  const _ = {...[1], ...[1]}; //# 16: compile-time error
        ^

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@kmillikin kmillikin

Labels
area-front-end Use area-front-end for front end / CFE / kernel format related issues.
Projects
None yet
Milestone
Dart 2.3
Development

No branches or pull requests
2 participants
@kmillikin @alexmarkov