| commit | 5b23b2c40cb0b69b7adf088d903e1db313ca9b1c | [log] [tgz] |
|---|---|---|
| author | Paul Berry <paulberry@google.com> | Wed Oct 16 19:01:53 2024 +0000 |
| committer | dart-internal-monorepo <dart-internal-monorepo@dart-ci-internal.iam.gserviceaccount.com> | Wed Oct 16 12:04:04 2024 -0700 |
| tree | 2451559daa0f938362ca75e1fece306eeec1c225 | |
| parent | 8e653e3fa4608b1919438f4ebe4ad2c78116eb4d [diff] |
analyzer: Fix null shorting of rewritten expressions. Background: null-shorting is a feature of Dart in which an expression might evaluate to `null` because a `null` was found deep in a subexpression, skipping evaluation of the rest of the expression. For example, in the expression `a?.b(c).d`, if `a` evaluates to `null`, execution will skip the evaluation of `c`, the method call to `b`, and the get of `d`, will be skipped, and the whole expression `a?.b(c).d` will evaluate to `null`. The analyzer needs to account for this by making the static type of `a?.b(c).d` nullable, even if the type of the `d` getter is not nullable. The analyzer's implementation of null shorting works like this: - When visiting a null-aware expression (such as `a?.b(c)` in the above example), the analyzer uses `NullShortableExpression.nullShortingTermination` to find the "null shorting termination expression". This is the expression that will evaluate to `null` if the null-shorting code path is taken (`a?.b(c).d` in the above example). - The null shorting termination expression is pushed onto the stack `ResolverVisitor._unfinishedNullShorts`. - After visiting an expression that might be a null shorting termination expression, the analyzer passes the node that was just visited to `ResolverVisitor.nullShortingTermination`, which checks whether it matches any nodes at the top of the `ResolverVisitor._unfinishedNullShorts` stack. If any nodes match, they are popped off the stack, and the static type of the null shorting termination expression is made nullable. A subtlety with this approach is that if the resolution process has rewritten the null shorting termination expression, then there are two expressions in play: the old one (from the original AST, before rewriting) and the new one (after rewriting). The node in `ResolverVisitor._unfinishedNullShorts` is the old one, because the code that pushes nodes onto the stack happens before rewrites. But the node that needs to have its static type changed is the new one, because that's the one that will wind up in the final resolved AST. In fact, trying to change the static type of the old node would lead to a crash, because the old node doesn't have a static type assigned. Previous to this CL, the analyzer dealt with this situation by having `ResolverVisitor.visitMethodInvocation` pass `discardType: true` to `ResolverVisitor.nullShortingTermination`. This disabled the logic for changing the type of the null shorting termination expression, which avoided a crash, but it meant that the type was never updated properly, leading to https://github.com/dart-lang/sdk/issues/56896. The proper solution is to pass both the old and new nodes to `ResolverVisitor.nullShortingTermination`. The old node is matched up against `ResolverVisitor._unfinishedNullShorts`, and the new node is used for marking the static type as nullable. As part of this fix, I've changed the return types of methods in `MethodInvocationResolver` from `FunctionExpressionInvocation?` to `FunctionExpressionInvocationImpl?`. This is a harmless change, since all these methods are private to the analyzer, and it avoids some type casts in `ResolverVisitor.nullShortingTermination`. I also added an assertion to `ResolverVisitor.nullShortingTermination` to verify that the correct rewritten node is passed in (this assertion relies on the `ResolverVisitor._replacements` expando, which is only populated when assertions are enabled). Adding this assertion exposed two other call sites that had to be updated (in `ResolverVisitor.visitIndexExpression` and `ResolverVisitor.visitPropertyAccess`). I've added additional analyzer tests to cover these cases. Fixes https://github.com/dart-lang/sdk/issues/56896. Bug: https://github.com/dart-lang/sdk/issues/56896 Change-Id: I8119a05b4d9f386b1129e5419b823a61677358c5 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/390560 Reviewed-by: Konstantin Shcheglov <scheglov@google.com> Commit-Queue: Paul Berry <paulberry@google.com> https://dart.googlesource.com/sdk/+/3546ba47d94cc526a01b5280476e3f1a0e148c61
Monorepo is:
With depot_tools installed and on your path, create a directory for your monorepo checkout and run these commands to create a gclient solution in that directory:
mkdir monorepo cd monorepo gclient config --unmanaged https://dart.googlesource.com/monorepo gclient sync -D
This gives you a checkout in the monorepo directory that contains:
monorepo/ DEPS - the DEPS used for this gclient checkout commits.json - the pinned commits for Dart, flutter/engine, and flutter/flutter tools/ - scripts used to create monorepo DEPS engine/src/ - the flutter/buildroot repo flutter/ - the flutter/engine repo out/ - the build directory, where Flutter engine builds are created third_party/ - Flutter dependencies checked out by DEPS dart/ - the Dart SDK checkout. third_party - Dart dependencies, also used by Flutter flutter/ - the flutter/flutter repo
Flutter's instructions for building the engine are at Compiling the engine
They can be followed closely, with a few changes:
goma_ctl ensure_start is sufficient.Example build commands that work on linux:
MONOREPO_PATH=$PWD if [[ ! $PATH =~ (^|:)$MONOREPO_PATH/flutter/bin(:|$) ]]; then PATH=$MONOREPO_PATH/flutter/bin:$PATH fi export GOMA_DIR=$(dirname $(command -v gclient))/.cipd_bin goma_ctl ensure_start pushd engine/src flutter/tools/gn --goma --no-prebuilt-dart-sdk --unoptimized --full-dart-sdk autoninja -C out/host_debug_unopt popd
The Flutter commands used to build and run apps will use the locally built Flutter engine and Dart SDK, instead of the one downloaded by the Flutter tool, if the --local-engine option is provided.
For example, to build and run the Flutter spinning square sample on the web platform,
MONOREPO_PATH=$PWD cd flutter/examples/layers flutter --local-engine=host_debug_unopt \ -d chrome run widgets/spinning_square.dart cd $MONOREPO_PATH
To build for desktop, specify the desktop platform device in flutter run as -d macos or -d linux or -d windows. You may also need to run the command
flutter create --platforms=windows,macos,linux
on existing apps, such as sample apps. New apps created with flutter create already include these support files. Details of desktop support are at Desktop Support for Flutter
Tests in the Flutter source tree can be run with the flutter test command, run in the directory of a package containing tests. For example:
MONOREPO_PATH=$PWD cd flutter/packages/flutter flutter test --local-engine=host_debug_unopt cd $MONOREPO_PATH
Please file an issue or email the dart-engprod team with any problems with or questions about using monorepo.
We will update this documentation to address them.
flutter commands may download the engine and Dart SDK files for the configured channel, even though they will be using the local engine and its SDK.gclient sync needs to be run in an administrator session, because some installed dependencies create symlinks.