Skip to content

[dev_compiler] Support enabling asserts for expression evaluation #43986

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.

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

Closed
annagrin opened this issue Oct 29, 2020 · 5 comments
Closed

[dev_compiler] Support enabling asserts for expression evaluation #43986

annagrin opened this issue Oct 29, 2020 · 5 comments
Labels
area-web-js Issues related to JavaScript support for Dart Web, including DDC, dart2js, and JS interop. P3 A lower priority bug or feature request web-dev-compiler

Comments

@annagrin
Copy link
Contributor

annagrin commented Oct 29, 2020

Ddc relies on source information during compilation for creating assert statements. This information is not available if the kernel is loaded from dill, as in expression compilation provided by the expression compilation worker.

Suggestion: store needed source text for asserts in kernel instead.
Alternative suggestion: source information (such as file and offsets) and invoke some post-failure step to add source text to the failure message.

See:

@devoncarew
Copy link
Member

@annagrin - when creating an issue on the sdk repo - if you know the area for it (like area-web for this item) - can you add the area label? It'll reduce the work for the issue triagers; thanks!

@devoncarew devoncarew added the area-web-js Issues related to JavaScript support for Dart Web, including DDC, dart2js, and JS interop. label Oct 30, 2020
@annagrin
Copy link
Contributor Author

Sure, thanks!

@annagrin annagrin added the P3 A lower priority bug or feature request label Oct 26, 2022
@nshahan
Copy link
Contributor

nshahan commented Dec 2, 2022

One reason that compiling an assertion requires the source is that flutter requested when assertions fail the error message includes the source code that failed. The VM does this, but I don't know if the VM does this for assertions written in debugger expressions. We could double check if they do and if so how do they get the source code?

I think another possibility here would be to just not include that information in the error message if there isn't a valid source location available. We could have a separate path when compiling assertions in the expression compiler that is more forgiving and not expect source location to be available. I think it would be understandable if you were to write a failing assertion expression in the debugger and you didn't get back source information when it failed. A message that just says something like "Assertion failed. Error from compiled expression."

@annagrin
Copy link
Contributor Author

annagrin commented Aug 3, 2023

Update: I am changing the DDC compiler code to be resilient to this failure - ie have some reporting with missing information. This allows us to enable asserts in the expression compiler and add tests in the SDK that match the CI bot configurations.

https://dart-review.googlesource.com/c/sdk/+/317448

Evaluating () { assert(false); return 0; } will now result in

Error: Assertion failed: <unknown source>:-1:-1
  BoolLiteral(false)
  is not true

Note that VM does something similar:

Unhandled exception:
'': Failed assertion: line -1: '<optimized out>': is not true.
#0      _AssertionError._doThrowNew (dart:core-patch/errors_patch.dart:51:61)
#1      _AssertionError._throwNew (dart:core-patch/errors_patch.dart:40:5)
#2      Eval.<anonymous closure> ()
#3      Eval ()

copybara-service bot pushed a commit that referenced this issue Aug 4, 2023
Closes: #53077
Towards: #43986
Change-Id: I60df13b8ff29a0865a45b3a48a97af0ce94460b7
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/317448
Reviewed-by: Nicholas Shahan <[email protected]>
Commit-Queue: Anna Gringauze <[email protected]>
@annagrin
Copy link
Contributor Author

annagrin commented Aug 4, 2023

@annagrin annagrin closed this as completed Aug 4, 2023
copybara-service bot pushed a commit that referenced this issue Mar 8, 2024
When assertions appear in a debugger expression they now have a
synthetic source location available. This also allows for the lookup
of the actual source.

Issue: #43986
Fixes: #54956
Change-Id: I34ae5f810593b40282929d02cb290de86603922f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/356287
Reviewed-by: Sigmund Cherem <[email protected]>
Commit-Queue: Nicholas Shahan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-web-js Issues related to JavaScript support for Dart Web, including DDC, dart2js, and JS interop. P3 A lower priority bug or feature request web-dev-compiler
Projects
None yet
Development

No branches or pull requests

4 participants