Fix assertion failure computing "why not promoted" for a public getter.

The data structure in the front end for recording which getter names
can be type promoted via "field promotion",
`fieldNonPromotabilityInfo`, is a field in the `SourceLibraryBuilder`
class. This means that each library tracks its own notion of which
getter names are promotable. This makes sense because only private
getter names are eligible for field promotion, and private getter
names can only be used to access declarations in the same library.

Prior to this commit, if the user tried to perform type promotion on a
_public_ getter name, the shared flow analysis logic would correctly
deem that public name non-promotable and reject the promotion. If the
front end then called `FlowAnalysis.whyNotPromoted` (which it
typically does to get details about failed type promotions that lead
to compile-time errors), flow analysis would then use
`FlowAnalysisOperations.whyPropertyIsNotPromotable` to query the
`fieldNonPromotabilityInfo` data structure.

Since `fieldNonPromotabilityInfo` is tracked separately for each
library, if the public getter referred to a field declared in some
other library, no information would be found, so
`FlowAnalysisOperations.whyPropertyIsNotPromotable` would return
`null`. This would lead flow analysis to incorrectly conclude that the
reason for the getter name being non-promotable was due to a conflict
with some other getter with the same name in the same library, so it
would return `PropertyNotPromotedForNonInherentReason` to the front
end. The front end would then iterate through
`fieldNonPromotabilityInfo` looking for conflicting getters, again
finding nothing. This led to an assertion failure, because it doesn't
make logical sense for a getter name to be non-promotale due to
conflicts if there are no conflicts. In production builds of the front
end (with assertions disabled), the behavior was that the compile-time
error would simply have no "why not promoted" context message (which
is fairly benign, but still undesirable).

This commit fixes the bug by modifying the shared logic for
`FlowAnalysis.whyNotPromoted` so that if the getter name in question
doesn't begin with `_` (meaning it's public), then it doesn't bother
calling `FlowAnalysisOperations.whyPropertyIsNotPromotable` at all,
because it knows the non-promotion reason is because the name is
public. As a result, the correct context message gets generated, and
there is no assertion failure.

Note that the language test included in this commit doesn't check that
the correct context message is generated, because the test
infrastructure isn't capable of testing context messages that point to
other files. But it does still serve to validate that the assertion no
longer fires.

Fixes https://github.com/dart-lang/sdk/issues/54777.

Change-Id: I697f55acad7c162bc5f49f2824c91d172497f344
Bug: https://github.com/dart-lang/sdk/issues/54777
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/349405
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Chloe Stefantsova <cstefantsova@google.com>
This commit is contained in:
Paul Berry 2024-01-31 15:40:38 +00:00 committed by Commit Queue
parent 226667e764
commit 2958754ed8
4 changed files with 55 additions and 5 deletions

View file

@ -5678,7 +5678,9 @@ class _FlowAnalysisImpl<Node extends Object, Statement extends Node,
Object? propertyMember = reference.propertyMember;
if (propertyMember != null) {
PropertyNonPromotabilityReason? whyNotPromotable =
operations.whyPropertyIsNotPromotable(propertyMember);
reference.propertyName.startsWith('_')
? operations.whyPropertyIsNotPromotable(propertyMember)
: PropertyNonPromotabilityReason.isNotPrivate;
_PropertySsaNode<Type>? ssaNode =
(reference.ssaNode as _PropertySsaNode<Type>).previousSsaNode;
List<List<Type>>? allPreviouslyPromotedTypes;

View file

@ -26,10 +26,10 @@ abstract interface class FlowAnalysisOperations<Variable extends Object,
///
/// This method is only called if a closure returned by
/// [FlowAnalysis.whyNotPromoted] is invoked, and the expression being queried
/// is a reference to a property that wasn't promoted; this typically means
/// that an error occurred and the client is attempting to produce a context
/// message to provide additional information about the error (i.e., that the
/// error happened due to failed promotion).
/// is a reference to a private property that wasn't promoted; this typically
/// means that an error occurred and the client is attempting to produce a
/// context message to provide additional information about the error (i.e.,
/// that the error happened due to failed promotion).
///
/// The client should return `null` if [property] was not promotable due to a
/// conflict with a field, getter, or noSuchMethod forwarder elsewhere in the

View file

@ -0,0 +1,38 @@
// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
// Tests that the appropriate "why not promoted" context messages are shown when
// field promotion fails due to the field being public.
//
// Both fields in the same library and in other libraries are exercised.
import 'why_not_promoted_public_lib.dart';
class ClassInSameLibrary {
final int? i;
// ^
// [context 1] 'i' refers to a public property so it couldn't be promoted. See http://dart.dev/go/non-promo-public-field
// [context 2] 'i' refers to a public property so it couldn't be promoted.
ClassInSameLibrary(this.i);
}
void testSameLibrary(ClassInSameLibrary c) {
if (c.i != null) {
c.i.isEven;
// ^^^^^^
// [analyzer 1] COMPILE_TIME_ERROR.UNCHECKED_USE_OF_NULLABLE_VALUE
// [cfe 2] Property 'isEven' cannot be accessed on 'int?' because it is potentially null.
}
}
void testOtherLibrary(ClassInOtherLibrary c) {
if (c.i != null) {
c.i.isEven;
// ^^^^^^
// [analyzer] COMPILE_TIME_ERROR.UNCHECKED_USE_OF_NULLABLE_VALUE
// [cfe] Property 'isEven' cannot be accessed on 'int?' because it is potentially null.
}
}
main() {}

View file

@ -0,0 +1,10 @@
// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
// Library imported by `why_not_promoted_public_error_test.dart`.
class ClassInOtherLibrary {
final int? i;
ClassInOtherLibrary(this.i);
}