Skip to content

Commit

Permalink
[stable] [_fe_analyzer_shared] Handle private names in exhaustiveness…
Browse files Browse the repository at this point in the history
… checking

Closes #52041

Cherry-pick: https://dart-review.googlesource.com/c/sdk/+/297462
Change-Id: I2bc2fc8fb38b30eb5e9c6e639c2603a5508dfec1
Fixes: #52254
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/302845
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
  • Loading branch information
stereotype441 authored and Commit Queue committed May 15, 2023
1 parent 7498681 commit 66fe70a
Show file tree
Hide file tree
Showing 16 changed files with 335 additions and 18 deletions.
28 changes: 28 additions & 0 deletions pkg/_fe_analyzer_shared/test/exhaustiveness/data/issue52041.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// Copyright (c) 2023, 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.

sealed class B {}

class C extends B {
final int _i;

C(this._i);
}

f(B b) {
/*
checkingOrder={B,C},
fields={_i:-},
subtypes={C},
type=B
*/
switch (b) {
/*space=C(_i: int)*/ case C(:var _i):
print('C($_i)');
}
}

main() {
f(C(0));
}
40 changes: 40 additions & 0 deletions pkg/_fe_analyzer_shared/test/exhaustiveness/data/private/main.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
// Copyright (c) 2023, 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.

import 'main_lib.dart';

class A {
num get _a => 42
int get _b => 42;
}

class C extends B {
}

exhaustiveA(A a) => /*
fields={_a:num},
type=A
*/switch (a) { A(: num _a) /*space=A(_a: num)*/=> 0, }

nonExhaustiveA(A a) => /*
error=non-exhaustive:A(_a: double()),
fields={_a:num},
type=A
*/switch (a) { A(: int _a) /*space=A(_a: int)*/=> 0, }

exhaustiveB(B b) => /*
fields={_b:int},
type=B
*/switch (b) { B(: int _b) /*space=B(_b: int)*/=> 0, }

exhaustiveC(C c) => /*
fields={_a:num},
type=C
*/switch (c) { C(: num _a) /*space=C(_a: num)*/=> 0, }

nonExhaustiveA(C c) => /*analyzer.
error=non-exhaustive:C(_a: double()),
fields={_a:num},
type=C
*/switch (c) { C(: int _a) /*analyzer.space=C(_a: int)*/=> 0, }
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// Copyright (c) 2023, 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.

import 'main.dart';

class B extends A {
int get _a => 42;
num get _b => 42;
}
3 changes: 2 additions & 1 deletion pkg/analyzer/lib/src/dart/constant/constant_verifier.dart
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,8 @@ class ConstantVerifier extends RecursiveAstVisitor<void> {
_currentLibrary.featureSet.isEnabled(Feature.non_nullable),
configuration: ConstantEvaluationConfiguration(),
),
_exhaustivenessCache = AnalyzerExhaustivenessCache(_typeSystem) {
_exhaustivenessCache =
AnalyzerExhaustivenessCache(_typeSystem, _currentLibrary) {
exhaustivenessDataForTesting = retainDataForTesting
? ExhaustivenessDataForTesting(_exhaustivenessCache)
: null;
Expand Down
13 changes: 10 additions & 3 deletions pkg/analyzer/lib/src/generated/exhaustiveness.dart
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,9 @@ class AnalyzerExhaustivenessCache extends ExhaustivenessCache<DartType,
InterfaceElement, EnumElement, FieldElement, DartObject> {
final TypeSystemImpl typeSystem;

AnalyzerExhaustivenessCache(this.typeSystem)
AnalyzerExhaustivenessCache(this.typeSystem, LibraryElement enclosingLibrary)
: super(
AnalyzerTypeOperations(typeSystem),
AnalyzerTypeOperations(typeSystem, enclosingLibrary),
const AnalyzerEnumOperations(),
AnalyzerSealedClassOperations(typeSystem));
}
Expand Down Expand Up @@ -165,10 +165,11 @@ class AnalyzerSealedClassOperations

class AnalyzerTypeOperations implements TypeOperations<DartType> {
final TypeSystemImpl _typeSystem;
final LibraryElement _enclosingLibrary;

final Map<InterfaceType, Map<Key, DartType>> _interfaceFieldTypesCaches = {};

AnalyzerTypeOperations(this._typeSystem);
AnalyzerTypeOperations(this._typeSystem, this._enclosingLibrary);

@override
DartType get boolType => _typeSystem.typeProvider.boolType;
Expand Down Expand Up @@ -322,11 +323,17 @@ class AnalyzerTypeOperations implements TypeOperations<DartType> {
fieldTypes.addAll(_getInterfaceFieldTypes(supertype));
}
for (PropertyAccessorElement accessor in type.accessors) {
if (accessor.isPrivate && accessor.library != _enclosingLibrary) {
continue;
}
if (accessor.isGetter && !accessor.isStatic) {
fieldTypes[NameKey(accessor.name)] = accessor.type.returnType;
}
}
for (MethodElement method in type.methods) {
if (method.isPrivate && method.library != _enclosingLibrary) {
continue;
}
if (!method.isStatic) {
fieldTypes[NameKey(method.name)] = method.type;
}
Expand Down
22 changes: 12 additions & 10 deletions pkg/front_end/lib/src/fasta/kernel/constant_evaluator.dart
Original file line number Diff line number Diff line change
Expand Up @@ -399,11 +399,11 @@ class ConstantsTransformer extends RemovingTransformer {
final bool errorOnUnevaluatedConstant;
final bool isLateLocalLoweringEnabled;

/// Cache used for checking exhaustiveness.
late final CfeExhaustivenessCache exhaustivenessCache;

final ExhaustivenessDataForTesting? _exhaustivenessDataForTesting;

/// Cache used for checking exhaustiveness.
CfeExhaustivenessCache? _exhaustivenessCache;

ConstantsTransformer(
Target target,
Map<String, String>? environmentDefines,
Expand Down Expand Up @@ -431,9 +431,7 @@ class ConstantsTransformer extends RemovingTransformer {
enableConstFunctions: enableConstFunctions,
errorOnUnevaluatedConstant: errorOnUnevaluatedConstant,
evaluationMode: evaluationMode),
_exhaustivenessDataForTesting = exhaustivenessDataForTesting {
exhaustivenessCache = new CfeExhaustivenessCache(constantEvaluator);
}
_exhaustivenessDataForTesting = exhaustivenessDataForTesting {}

/// Whether to preserve constant [Field]s. All use-sites will be rewritten.
bool get keepFields => backend.keepFields;
Expand All @@ -452,6 +450,9 @@ class ConstantsTransformer extends RemovingTransformer {
_staticTypeContext =
new StaticTypeContext.forAnnotations(library, typeEnvironment);

_exhaustivenessCache =
new CfeExhaustivenessCache(constantEvaluator, library);

transformAnnotations(library.annotations, library);

transformLibraryDependencyList(library.dependencies, library);
Expand All @@ -471,6 +472,7 @@ class ConstantsTransformer extends RemovingTransformer {
});
}
_staticTypeContext = null;
_exhaustivenessCache = null;
}

@override
Expand Down Expand Up @@ -1537,22 +1539,22 @@ class ConstantsTransformer extends RemovingTransformer {
required bool hasDefault,
required bool mustBeExhaustive,
required bool isSwitchExpression}) {
StaticType type = exhaustivenessCache.getStaticType(
StaticType type = _exhaustivenessCache!.getStaticType(
// Treat invalid types as empty.
expressionType is InvalidType
? const NeverType.nonNullable()
: expressionType);
List<Space> cases = [];
PatternConverter patternConverter = new PatternConverter(
exhaustivenessCache, staticTypeContext,
_exhaustivenessCache!, staticTypeContext,
hasPrimitiveEquality: (Constant constant) => constantEvaluator
.hasPrimitiveEqual(constant, staticTypeContext: staticTypeContext));
for (PatternGuard patternGuard in patternGuards) {
cases.add(patternConverter.createRootSpace(type, patternGuard.pattern,
hasGuard: patternGuard.guard != null));
}
List<ExhaustivenessError> errors =
reportErrors(exhaustivenessCache, type, cases);
reportErrors(_exhaustivenessCache!, type, cases);
List<ExhaustivenessError>? reportedErrors;
if (_exhaustivenessDataForTesting != null) {
reportedErrors = [];
Expand Down Expand Up @@ -1592,7 +1594,7 @@ class ConstantsTransformer extends RemovingTransformer {
}
}
if (_exhaustivenessDataForTesting != null) {
_exhaustivenessDataForTesting!.objectFieldLookup ??= exhaustivenessCache;
_exhaustivenessDataForTesting!.objectFieldLookup ??= _exhaustivenessCache;
_exhaustivenessDataForTesting!.switchResults[replacement] =
new ExhaustivenessResult(type, cases,
patternGuards.map((c) => c.fileOffset).toList(), reportedErrors!);
Expand Down
11 changes: 7 additions & 4 deletions pkg/front_end/lib/src/fasta/kernel/exhaustiveness.dart
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,9 @@ class ExhaustivenessResult {

class CfeTypeOperations implements TypeOperations<DartType> {
final TypeEnvironment _typeEnvironment;
final Library _enclosingLibrary;

CfeTypeOperations(this._typeEnvironment);
CfeTypeOperations(this._typeEnvironment, this._enclosingLibrary);

ClassHierarchy get _classHierarchy => _typeEnvironment.hierarchy;

Expand Down Expand Up @@ -122,7 +123,7 @@ class CfeTypeOperations implements TypeOperations<DartType> {
Map<Class, Substitution> substitutions = {};
for (Member member
in _classHierarchy.getInterfaceMembers(type.classNode)) {
if (member.name.isPrivate) {
if (member.name.isPrivate && member.name.library != _enclosingLibrary) {
continue;
}
DartType? fieldType;
Expand Down Expand Up @@ -396,10 +397,12 @@ class CfeExhaustivenessCache
extends ExhaustivenessCache<DartType, Class, Class, Field, Constant> {
final TypeEnvironment typeEnvironment;

CfeExhaustivenessCache(ConstantEvaluator constantEvaluator)
CfeExhaustivenessCache(
ConstantEvaluator constantEvaluator, Library enclosingLibrary)
: typeEnvironment = constantEvaluator.typeEnvironment,
super(
new CfeTypeOperations(constantEvaluator.typeEnvironment),
new CfeTypeOperations(
constantEvaluator.typeEnvironment, enclosingLibrary),
new CfeEnumOperations(constantEvaluator),
new CfeSealedClassOperations(constantEvaluator.typeEnvironment));
}
Expand Down
22 changes: 22 additions & 0 deletions pkg/front_end/testcases/patterns/exhaustiveness/issue52041.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// Copyright (c) 2023, 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.

sealed class B {}

class C extends B {
final int _i;

C(this._i);
}

f(B b) {
switch (b) {
case C(:var _i):
print('C($_i)');
}
}

main() {
f(C(0));
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
library /*isNonNullableByDefault*/;
import self as self;
import "dart:core" as core;

abstract sealed class B extends core::Object {
synthetic constructor •() → self::B
: super core::Object::•()
;
}
class C extends self::B {
final field core::int _i;
constructor •(core::int _i) → self::C
: self::C::_i = _i, super self::B::•()
;
}
static method f(self::B b) → dynamic {
#L1:
{
final synthesized self::B #0#0 = b;
{
hoisted core::int _i;
if(#0#0 is{ForNonNullableByDefault} self::C && (let final dynamic #t1 = _i = #0#0{self::C}.{self::C::_i}{core::int} in true)) {
{
core::print("C(${_i})");
}
}
}
}
}
static method main() → dynamic {
self::f(new self::C::•(0));
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
library /*isNonNullableByDefault*/;
import self as self;
import "dart:core" as core;

abstract sealed class B extends core::Object {
synthetic constructor •() → self::B
: super core::Object::•()
;
}
class C extends self::B {
final field core::int _i;
constructor •(core::int _i) → self::C
: self::C::_i = _i, super self::B::•()
;
}
static method f(self::B b) → dynamic {
#L1:
{
final synthesized self::B #0#0 = b;
{
hoisted core::int _i;
if(#0#0 is{ForNonNullableByDefault} self::C && (let final core::int #t1 = _i = #0#0{self::C}.{self::C::_i}{core::int} in true)) {
{
core::print("C(${_i})");
}
}
}
}
}
static method main() → dynamic {
self::f(new self::C::•(0));
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
sealed class B {}

class C extends B {
final int _i;
C(this._i);
}

f(B b) {}
main() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
class C extends B {
C(this._i);
final int _i;
}

f(B b) {}
main() {}

sealed class B {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
library /*isNonNullableByDefault*/;
import self as self;
import "dart:core" as core;
import "dart:_internal" as _in;

abstract sealed class B extends core::Object {
synthetic constructor •() → self::B
: super core::Object::•()
;
}
class C extends self::B {
final field core::int _i;
constructor •(core::int _i) → self::C
: self::C::_i = _i, super self::B::•()
;
}
static method f(self::B b) → dynamic {
#L1:
{
final synthesized self::B #0#0 = b;
{
hoisted core::int _i;
if(#0#0 is{ForNonNullableByDefault} self::C && (let final dynamic #t1 = _i = #0#0{self::C}.{self::C::_i}{core::int} in true)) {
{
core::print("C(${_i})");
}
}
break #L1;
}
throw new _in::ReachabilityError::•("`null` encountered as case in a switch statement with a non-nullable type.");
}
}
static method main() → dynamic {
self::f(new self::C::•(0));
}
Loading

0 comments on commit 66fe70a

Please sign in to comment.