Augment output in dart compile to clarify null safety compilation mode #44234

vsmenon · 2020-11-17T17:17:37Z

I got confused when retrying a migration. I'd accidently left a @dart = 2.10 in my entry point but migrated the rest of the app and the pubspec.

Might be worth emitting a warning that the mode disagrees with the pubspec.

Also not sure if this is best done in CFE or flutter tool - @mit-mit @johnniwinther @jonahwilliams

The text was updated successfully, but these errors were encountered:

johnniwinther · 2020-11-17T17:20:44Z

The CFE never reads the pubspec.yaml. Only the package_config.json (through package:package_config).

jonahwilliams · 2020-11-17T18:31:15Z

Should this be a warning? Maybe the AI is to make it more obvious what mode we're running in?

johnniwinther · 2020-12-02T07:58:01Z

@mit-mit What are your thoughts on this?

mit-mit · 2020-12-03T10:22:15Z

From a discussion in the language team, we think this should not be a warning shown when running dart code (e.g. running dart bin/main.dart; that is potentially breaking. We also don't think it should be shown during analysis, as that could easily get annoying (the developer opts out, and is then immediately hit with warning).

But, we still think it's worth adjusting the output of commands that build release-mode artifacts to state which form of null safety was used. Concretely we'd like to suggest we adjust the output of dart compile and flutter build, so that when the SDK constraint is 2.12 or above, we add output like this:

$ dart compile exe bin/myapp.dart
Compiled exe with sound|sound null safety: /Users/mit/tmp/myapp/bin/myapp.exe

$ flutter build apk
Compiling Dart code with sound|unsound null safety.
Running Gradle task 'assembleRelease'... Done                     113.6s
✓ Built build/app/outputs/flutter-apk/app-release.apk (15.4MB).

WDYT @jonahwilliams @bkonyi @devoncarew ?

devoncarew · 2020-12-07T17:28:42Z

Consider highlighting the unsound version:

Compiling Dart code with sound null safety.

vs.

Compiling Dart code with unsound null safety.

devoncarew · 2020-12-07T17:29:39Z

Or even:

Compiling Dart code with unsound null safety (see dart.dev/... for more information).

bkonyi · 2020-12-07T18:09:16Z

I'm fairly certain I saw flutter run output a message about running with unsound null safety last week. We should have a way for users to check whether or not they're running with sound/unsound null safety even when running against the JIT (maybe add a new flag or put it behind the --verbose option?). In general though, I'm supportive of adding this as output for commands that generate artifacts.

zanderso · 2020-12-08T16:27:36Z

The proposal in #44234 (comment) makes sense to me. Please file an issue in the Flutter issue tracker for the work needed on the Flutter tool. Thanks!

mit-mit · 2020-12-14T17:32:13Z

Thanks @zanderso, opened flutter/flutter#72279 for the warning in flutter build

mit-mit · 2020-12-14T17:56:07Z

Updating the present issue to track the change in dart compile:

$ dart compile exe bin/myapp.dart
Compiled exe with sound|sound null safety: /Users/mit/tmp/myapp/bin/myapp.exe

bkonyi · 2020-12-21T20:42:40Z

I'm uncertain as to which level in the stack should be outputting this information, but I'm thinking it needs to be done in each compilation pipeline. dart compile consists of 3 different compilation pipelines: dart2js, dart2native, and snapshot generation done by the standalone VM, each of which should probably be responsible for detecting what type of null safety is being used and outputting the correct message. I can do this for the VM, but someone more familiar with dart2js and dart2native should look into adding this there.

cc/ @dcharkes for dart2native and @sigmundch for dart2js.

sigmundch · 2020-12-21T22:58:12Z

Agree this cannot be handled in dart-cli, but in each underlying compiler since the information necessary for this is available much later in time.

One alternative could be to share the logic in the CFE. To clarify on the earlier comments, this wouldn't require reading the pubspec.yaml, it would just mean to report a warning if the main entrypoint is in a library that is using legacy semantics. Doing this in the CFE may be nice since the CFE already reports errors that are similar in nature when the app is in sound mode (e.g. complaining if you import an opted-out library).

Currently we already have Component.setMainMethodAndMode - maybe introducing an indirection to calls to this method could be enough to always report a warning if main is from a legacy library? @johnniwinther what do you think?

johnniwinther · 2020-12-25T13:05:09Z

Generally the CFE doesn't know the context for its invocation, meaning that any messaging provided for one context will be misleading/wrong/confusing for another. Besides this case, we for instance mention updating pubspec.yaml in some error message and that doesn't make sense if the package_config.json (which is the file actually read by the CFE) is not generated by pub.

For this reason I'd like to add support for passing an identification of this context to CFE and use this to guide its messaging.

bkonyi · 2021-01-05T21:11:10Z

@johnniwinther it sounds like you have a good handle on what needs to be done here. Can I get you to drive this work?

johnniwinther · 2021-01-06T07:10:35Z

I will.

johnniwinther · 2021-01-06T12:53:18Z

Should the message be reported as a warning (i.e. with colors and "Warning: " prefix) ?

mit-mit · 2021-01-06T13:28:30Z

No, I think I would classify this as an information message, not a warning.

mit-mit · 2021-01-08T13:52:47Z

Flutter work (flutter/flutter#72279) is now complete.

mit-mit · 2021-01-14T10:39:18Z

Code review in https://dart-review.googlesource.com/c/sdk/+/178982

This adds a new messages kind 'info' to the CFE for showing general information during compilation. A 'configuration' options is added to `CompilerOptions` for telling the CFE how it is run. The configuration 'compile' is added for when the CFE is invoked to produces an "executable" as when running `dart compile`. When configuration is set, the CFE emits an info message about the null safety compilation mode. Support for `dart compile exe` and `dart compile js` is added in this CL. Support for `dart compile kernel|app-jit|aot` is not included. In response to #44234 TEST=pkg/dartdev/test/commands/compile_test.dart Change-Id: I08f51e2a3f5ad4841c4d703bcd266b7afb63c7c6 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/178982 Commit-Queue: Johnni Winther <johnniwinther@google.com> Reviewed-by: Jens Johansen <jensj@google.com>

This reverts commit f8b0d26. Reason for revert: Multiple test failures in app-jit and simarm(64) Original change's description: > [vm] Pass snapshot flag to kernel_service > > Enable reporting of null safety compilation mode when running > `dart compile aot-snapshot`, `dart compile jit-snapshot`, > and `dart compile kernel`. > > Closes #44234 > > TEST=pkg/dartdev/test/commands/compile_test.dart > > Change-Id: I0d4b35c6ccb4167c0c7539a4eb24a5139e29cf53 > Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/178990 > Commit-Queue: Johnni Winther <johnniwinther@google.com> > Reviewed-by: Ben Konyi <bkonyi@google.com> TBR=bkonyi@google.com,johnniwinther@google.com Change-Id: I83aeaa8620c640d02d5ccfd1fe8112d8209a9ad4 No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/179773 Auto-Submit: Aske Simon Christensen <askesc@google.com> Reviewed-by: Johnni Winther <johnniwinther@google.com> Commit-Queue: Johnni Winther <johnniwinther@google.com>

askeksa-google · 2021-01-19T10:26:04Z

Reopening due to revert.

johnniwinther · 2021-01-19T10:28:29Z

@bkonyi can you take a look at this? - the app-jit/simarm failures are bit out of my scope.

vsmenon · 2021-01-19T16:42:26Z

@bkonyi @a-siva - does this look fixable soon? Trying to land this week.

bkonyi · 2021-01-19T20:41:58Z

Yes, I'll take a look.

bkonyi · 2021-01-19T21:10:42Z

@johnniwinther it looks like most of these failures can just be approved as the tests are failing due to stdout not matching the test expectations. However, the app_jitk-linux-debug-x64 assertion failures don't look good. @alexmarkov might be the right person to look at that since it's in the compiler.

bkonyi · 2021-01-19T21:34:16Z

Relanding the reverted change will be blocked until #44714 is resolved.

mit-mit · 2021-01-20T09:11:17Z

@johnniwinther as discussed yesterday, we should rephrase the 'unsound' message a bit, as it is shown both for projects with a 2.12 constraint & an entry-point opt-out, and for projects with a 2.9 constraint. I suggest:

- "Compiling with sound null safety" (everything is 2.12) or
- "Compiling without sound null safety" (something isn't 2.12)

In response to #44234 (comment) Change-Id: I88ab66199217e817edead394ccc74a3f3b49ba15 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/180143 Reviewed-by: Michael Thomsen <mit@google.com> Commit-Queue: Johnni Winther <johnniwinther@google.com>

mit-mit · 2021-01-20T16:14:52Z

"Compiling without sound null safety" UI string change landed in 134a437

franklinyow · 2021-01-21T02:03:36Z

#44714 is closed

johnniwinther · 2021-01-21T18:22:29Z

The reverted CL is ready to reland: https://dart-review.googlesource.com/c/sdk/+/180187

vsmenon added the NNBD Issues related to NNBD Release label Nov 17, 2020

keertip added the area-front-end Use area-front-end for front end / CFE / kernel format related issues. label Nov 18, 2020

johnniwinther added the P2 A bug or feature request we're likely to work on label Dec 2, 2020

mit-mit mentioned this issue Dec 14, 2020

Augment flutter build output to state null safety compilation mode flutter/flutter#72279

Closed

mit-mit changed the title ~~If pubspec is opted into null safety but entry is opted out, no warning.~~ Augment output in dart compile to clarify null safety compilation mode Dec 14, 2020

mit-mit added area-dart-cli Use area-dart-cli for issues related to the 'dart' command like tool. P1 A high priority bug; for example, a single project is unusable or has many test failures and removed P2 A bug or feature request we're likely to work on labels Dec 14, 2020

mit-mit assigned bkonyi Dec 14, 2020

franklinyow added this to the January 2021 milestone Jan 5, 2021

johnniwinther self-assigned this Jan 6, 2021

bkonyi removed their assignment Jan 6, 2021

franklinyow added the type-enhancement A request for a change that isn't a bug label Jan 8, 2021

dart-bot closed this as completed in f8b0d26 Jan 18, 2021

askeksa-google reopened this Jan 19, 2021

johnniwinther assigned bkonyi Jan 19, 2021

bkonyi mentioned this issue Jan 19, 2021

AppJIT Assertion Failure: ../../runtime/vm/object.h: 4177: error: expected: !IsOriginal() || is_static() || thread->IsMutatorThread() || thread->IsAtSafepoint() #44714

Closed

mit-mit mentioned this issue Jan 20, 2021

Rephrase null safety mode flutter/flutter#74306

Merged

dart-bot closed this as completed in 55b34a4 Jan 21, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Augment output in dart compile to clarify null safety compilation mode #44234

Augment output in dart compile to clarify null safety compilation mode #44234

vsmenon commented Nov 17, 2020

johnniwinther commented Nov 17, 2020

jonahwilliams commented Nov 17, 2020

johnniwinther commented Dec 2, 2020

mit-mit commented Dec 3, 2020

devoncarew commented Dec 7, 2020

devoncarew commented Dec 7, 2020

bkonyi commented Dec 7, 2020

zanderso commented Dec 8, 2020

mit-mit commented Dec 14, 2020 •

edited

Loading

mit-mit commented Dec 14, 2020 •

edited

Loading

bkonyi commented Dec 21, 2020 •

edited

Loading

sigmundch commented Dec 21, 2020

johnniwinther commented Dec 25, 2020

bkonyi commented Jan 5, 2021

johnniwinther commented Jan 6, 2021

johnniwinther commented Jan 6, 2021

mit-mit commented Jan 6, 2021

mit-mit commented Jan 8, 2021

mit-mit commented Jan 14, 2021

askeksa-google commented Jan 19, 2021

johnniwinther commented Jan 19, 2021

vsmenon commented Jan 19, 2021

bkonyi commented Jan 19, 2021

bkonyi commented Jan 19, 2021

bkonyi commented Jan 19, 2021

mit-mit commented Jan 20, 2021

mit-mit commented Jan 20, 2021

franklinyow commented Jan 21, 2021

johnniwinther commented Jan 21, 2021

Augment output in dart compile to clarify null safety compilation mode #44234

Augment output in dart compile to clarify null safety compilation mode #44234

Comments

vsmenon commented Nov 17, 2020

johnniwinther commented Nov 17, 2020

jonahwilliams commented Nov 17, 2020

johnniwinther commented Dec 2, 2020

mit-mit commented Dec 3, 2020

devoncarew commented Dec 7, 2020

devoncarew commented Dec 7, 2020

bkonyi commented Dec 7, 2020

zanderso commented Dec 8, 2020

mit-mit commented Dec 14, 2020 • edited Loading

mit-mit commented Dec 14, 2020 • edited Loading

bkonyi commented Dec 21, 2020 • edited Loading

sigmundch commented Dec 21, 2020

johnniwinther commented Dec 25, 2020

bkonyi commented Jan 5, 2021

johnniwinther commented Jan 6, 2021

johnniwinther commented Jan 6, 2021

mit-mit commented Jan 6, 2021

mit-mit commented Jan 8, 2021

mit-mit commented Jan 14, 2021

askeksa-google commented Jan 19, 2021

johnniwinther commented Jan 19, 2021

vsmenon commented Jan 19, 2021

bkonyi commented Jan 19, 2021

bkonyi commented Jan 19, 2021

bkonyi commented Jan 19, 2021

mit-mit commented Jan 20, 2021

mit-mit commented Jan 20, 2021

franklinyow commented Jan 21, 2021

johnniwinther commented Jan 21, 2021

mit-mit commented Dec 14, 2020 •

edited

Loading

mit-mit commented Dec 14, 2020 •

edited

Loading

bkonyi commented Dec 21, 2020 •

edited

Loading