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.

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

Kernel output contains an overly broad type when lowering an expression that's unnecessarily null-aware due to null shorting #59636

Closed
stereotype441 opened this issue Nov 29, 2024 · 1 comment
Assignees
Labels
area-front-end Use area-front-end for front end / CFE / kernel format related issues. cfe-encodings Encoding related CFE issues. cfe-optimizations CFE optimizations front-end-expression-compilation

Comments

@stereotype441
Copy link
Member

Consider the following null-aware expression (taken from pkg/front_end/testcases/nnbd/null_shorting.dart):

n1?.nonNullable1Method()?.nonNullable1Method();

Where n1 has type Class1? and Class1.nonNullable1Method has return type Class1. Note that the second ?. is not necessary (because, thanks to null shorting, it only executes when n1 != null); therefore it should, in principle, be possible to optimize it to . during compilation.

The kernel generated for this expression is (line breaks added for clarity):

    let final self::Class1? #t80 = n1
    in #t80 == null
         ?{self::Class1?} null
         : let final self::Class1? #t81 =
             #t80{self::Class1}.{self::Class1::nonNullable1Method}
               (){() → self::Class1}
           in #t81 == null
                ?{self::Class1?} null
                : #t81{self::Class1}.{self::Class1::nonNullable1Method}
                    (){() → self::Class1};

Although this has correct runtime semantics, the type of #t81 is unnecessarily broad. It could be self::Class1 instead of self::Class1?, since the initializer expression is the value returned by nonNullable1Method, which has return type Class1.

I suspect that if the type of #t81 were corrected, it would improve the chances of back-ends detecting that the null check #t81 == null is unnecessary (and thus performing the correct optimization).

@stereotype441 stereotype441 added area-front-end Use area-front-end for front end / CFE / kernel format related issues. front-end-expression-compilation labels Nov 29, 2024
@johnniwinther johnniwinther added cfe-encodings Encoding related CFE issues. cfe-optimizations CFE optimizations labels Nov 29, 2024
@stereotype441
Copy link
Member Author

I discovered this issue while exploring ways to share more of the logic for analyzing null-shorting expressions between the analyzer and CFE, so to unblock that work, I've gone ahead and prototyped a fix: https://dart-review.googlesource.com/c/sdk/+/398121

@stereotype441 stereotype441 self-assigned this Nov 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-front-end Use area-front-end for front end / CFE / kernel format related issues. cfe-encodings Encoding related CFE issues. cfe-optimizations CFE optimizations front-end-expression-compilation
Projects
None yet
Development

No branches or pull requests

2 participants