From 66fe70a277d1f9fa4e97ba69bb30a046fe3b2ba4 Mon Sep 17 00:00:00 2001 From: Paul Berry Date: Mon, 15 May 2023 20:03:23 +0000 Subject: [PATCH] [stable] [_fe_analyzer_shared] Handle private names in exhaustiveness checking Closes #52041 Cherry-pick: https://dart-review.googlesource.com/c/sdk/+/297462 Change-Id: I2bc2fc8fb38b30eb5e9c6e639c2603a5508dfec1 Fixes: https://github.com/dart-lang/sdk/issues/52254 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/302845 Reviewed-by: Johnni Winther Commit-Queue: Paul Berry --- .../test/exhaustiveness/data/issue52041.dart | 28 +++++++++++++ .../exhaustiveness/data/private/main.dart | 40 +++++++++++++++++++ .../exhaustiveness/data/private/main_lib.dart | 10 +++++ .../src/dart/constant/constant_verifier.dart | 3 +- .../lib/src/generated/exhaustiveness.dart | 13 ++++-- .../src/fasta/kernel/constant_evaluator.dart | 22 +++++----- .../lib/src/fasta/kernel/exhaustiveness.dart | 11 +++-- .../patterns/exhaustiveness/issue52041.dart | 22 ++++++++++ .../issue52041.dart.strong.expect | 32 +++++++++++++++ .../issue52041.dart.strong.transformed.expect | 32 +++++++++++++++ .../issue52041.dart.textual_outline.expect | 9 +++++ ...52041.dart.textual_outline_modelled.expect | 9 +++++ .../issue52041.dart.weak.expect | 35 ++++++++++++++++ .../issue52041.dart.weak.modular.expect | 35 ++++++++++++++++ .../issue52041.dart.weak.outline.expect | 17 ++++++++ .../issue52041.dart.weak.transformed.expect | 35 ++++++++++++++++ 16 files changed, 335 insertions(+), 18 deletions(-) create mode 100644 pkg/_fe_analyzer_shared/test/exhaustiveness/data/issue52041.dart create mode 100644 pkg/_fe_analyzer_shared/test/exhaustiveness/data/private/main.dart create mode 100644 pkg/_fe_analyzer_shared/test/exhaustiveness/data/private/main_lib.dart create mode 100644 pkg/front_end/testcases/patterns/exhaustiveness/issue52041.dart create mode 100644 pkg/front_end/testcases/patterns/exhaustiveness/issue52041.dart.strong.expect create mode 100644 pkg/front_end/testcases/patterns/exhaustiveness/issue52041.dart.strong.transformed.expect create mode 100644 pkg/front_end/testcases/patterns/exhaustiveness/issue52041.dart.textual_outline.expect create mode 100644 pkg/front_end/testcases/patterns/exhaustiveness/issue52041.dart.textual_outline_modelled.expect create mode 100644 pkg/front_end/testcases/patterns/exhaustiveness/issue52041.dart.weak.expect create mode 100644 pkg/front_end/testcases/patterns/exhaustiveness/issue52041.dart.weak.modular.expect create mode 100644 pkg/front_end/testcases/patterns/exhaustiveness/issue52041.dart.weak.outline.expect create mode 100644 pkg/front_end/testcases/patterns/exhaustiveness/issue52041.dart.weak.transformed.expect diff --git a/pkg/_fe_analyzer_shared/test/exhaustiveness/data/issue52041.dart b/pkg/_fe_analyzer_shared/test/exhaustiveness/data/issue52041.dart new file mode 100644 index 000000000000..47c603ebd138 --- /dev/null +++ b/pkg/_fe_analyzer_shared/test/exhaustiveness/data/issue52041.dart @@ -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)); +} diff --git a/pkg/_fe_analyzer_shared/test/exhaustiveness/data/private/main.dart b/pkg/_fe_analyzer_shared/test/exhaustiveness/data/private/main.dart new file mode 100644 index 000000000000..31be986e20aa --- /dev/null +++ b/pkg/_fe_analyzer_shared/test/exhaustiveness/data/private/main.dart @@ -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, } diff --git a/pkg/_fe_analyzer_shared/test/exhaustiveness/data/private/main_lib.dart b/pkg/_fe_analyzer_shared/test/exhaustiveness/data/private/main_lib.dart new file mode 100644 index 000000000000..bc2548a9f545 --- /dev/null +++ b/pkg/_fe_analyzer_shared/test/exhaustiveness/data/private/main_lib.dart @@ -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; +} diff --git a/pkg/analyzer/lib/src/dart/constant/constant_verifier.dart b/pkg/analyzer/lib/src/dart/constant/constant_verifier.dart index 3c556d28cbae..0d755047be0c 100644 --- a/pkg/analyzer/lib/src/dart/constant/constant_verifier.dart +++ b/pkg/analyzer/lib/src/dart/constant/constant_verifier.dart @@ -94,7 +94,8 @@ class ConstantVerifier extends RecursiveAstVisitor { _currentLibrary.featureSet.isEnabled(Feature.non_nullable), configuration: ConstantEvaluationConfiguration(), ), - _exhaustivenessCache = AnalyzerExhaustivenessCache(_typeSystem) { + _exhaustivenessCache = + AnalyzerExhaustivenessCache(_typeSystem, _currentLibrary) { exhaustivenessDataForTesting = retainDataForTesting ? ExhaustivenessDataForTesting(_exhaustivenessCache) : null; diff --git a/pkg/analyzer/lib/src/generated/exhaustiveness.dart b/pkg/analyzer/lib/src/generated/exhaustiveness.dart index c38871fddaf3..08eec6ff4220 100644 --- a/pkg/analyzer/lib/src/generated/exhaustiveness.dart +++ b/pkg/analyzer/lib/src/generated/exhaustiveness.dart @@ -65,9 +65,9 @@ class AnalyzerExhaustivenessCache extends ExhaustivenessCache { final TypeSystemImpl typeSystem; - AnalyzerExhaustivenessCache(this.typeSystem) + AnalyzerExhaustivenessCache(this.typeSystem, LibraryElement enclosingLibrary) : super( - AnalyzerTypeOperations(typeSystem), + AnalyzerTypeOperations(typeSystem, enclosingLibrary), const AnalyzerEnumOperations(), AnalyzerSealedClassOperations(typeSystem)); } @@ -165,10 +165,11 @@ class AnalyzerSealedClassOperations class AnalyzerTypeOperations implements TypeOperations { final TypeSystemImpl _typeSystem; + final LibraryElement _enclosingLibrary; final Map> _interfaceFieldTypesCaches = {}; - AnalyzerTypeOperations(this._typeSystem); + AnalyzerTypeOperations(this._typeSystem, this._enclosingLibrary); @override DartType get boolType => _typeSystem.typeProvider.boolType; @@ -322,11 +323,17 @@ class AnalyzerTypeOperations implements TypeOperations { 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; } diff --git a/pkg/front_end/lib/src/fasta/kernel/constant_evaluator.dart b/pkg/front_end/lib/src/fasta/kernel/constant_evaluator.dart index 31f78c5d50af..6374b3ad162c 100644 --- a/pkg/front_end/lib/src/fasta/kernel/constant_evaluator.dart +++ b/pkg/front_end/lib/src/fasta/kernel/constant_evaluator.dart @@ -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? environmentDefines, @@ -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; @@ -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); @@ -471,6 +472,7 @@ class ConstantsTransformer extends RemovingTransformer { }); } _staticTypeContext = null; + _exhaustivenessCache = null; } @override @@ -1537,14 +1539,14 @@ 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 cases = []; PatternConverter patternConverter = new PatternConverter( - exhaustivenessCache, staticTypeContext, + _exhaustivenessCache!, staticTypeContext, hasPrimitiveEquality: (Constant constant) => constantEvaluator .hasPrimitiveEqual(constant, staticTypeContext: staticTypeContext)); for (PatternGuard patternGuard in patternGuards) { @@ -1552,7 +1554,7 @@ class ConstantsTransformer extends RemovingTransformer { hasGuard: patternGuard.guard != null)); } List errors = - reportErrors(exhaustivenessCache, type, cases); + reportErrors(_exhaustivenessCache!, type, cases); List? reportedErrors; if (_exhaustivenessDataForTesting != null) { reportedErrors = []; @@ -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!); diff --git a/pkg/front_end/lib/src/fasta/kernel/exhaustiveness.dart b/pkg/front_end/lib/src/fasta/kernel/exhaustiveness.dart index ea481b106478..91d39d3c1676 100644 --- a/pkg/front_end/lib/src/fasta/kernel/exhaustiveness.dart +++ b/pkg/front_end/lib/src/fasta/kernel/exhaustiveness.dart @@ -46,8 +46,9 @@ class ExhaustivenessResult { class CfeTypeOperations implements TypeOperations { final TypeEnvironment _typeEnvironment; + final Library _enclosingLibrary; - CfeTypeOperations(this._typeEnvironment); + CfeTypeOperations(this._typeEnvironment, this._enclosingLibrary); ClassHierarchy get _classHierarchy => _typeEnvironment.hierarchy; @@ -122,7 +123,7 @@ class CfeTypeOperations implements TypeOperations { Map substitutions = {}; for (Member member in _classHierarchy.getInterfaceMembers(type.classNode)) { - if (member.name.isPrivate) { + if (member.name.isPrivate && member.name.library != _enclosingLibrary) { continue; } DartType? fieldType; @@ -396,10 +397,12 @@ class CfeExhaustivenessCache extends ExhaustivenessCache { 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)); } diff --git a/pkg/front_end/testcases/patterns/exhaustiveness/issue52041.dart b/pkg/front_end/testcases/patterns/exhaustiveness/issue52041.dart new file mode 100644 index 000000000000..e83e5cc8990e --- /dev/null +++ b/pkg/front_end/testcases/patterns/exhaustiveness/issue52041.dart @@ -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)); +} diff --git a/pkg/front_end/testcases/patterns/exhaustiveness/issue52041.dart.strong.expect b/pkg/front_end/testcases/patterns/exhaustiveness/issue52041.dart.strong.expect new file mode 100644 index 000000000000..22dd0ed13164 --- /dev/null +++ b/pkg/front_end/testcases/patterns/exhaustiveness/issue52041.dart.strong.expect @@ -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)); +} diff --git a/pkg/front_end/testcases/patterns/exhaustiveness/issue52041.dart.strong.transformed.expect b/pkg/front_end/testcases/patterns/exhaustiveness/issue52041.dart.strong.transformed.expect new file mode 100644 index 000000000000..0868e452a4e9 --- /dev/null +++ b/pkg/front_end/testcases/patterns/exhaustiveness/issue52041.dart.strong.transformed.expect @@ -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)); +} diff --git a/pkg/front_end/testcases/patterns/exhaustiveness/issue52041.dart.textual_outline.expect b/pkg/front_end/testcases/patterns/exhaustiveness/issue52041.dart.textual_outline.expect new file mode 100644 index 000000000000..55378917b754 --- /dev/null +++ b/pkg/front_end/testcases/patterns/exhaustiveness/issue52041.dart.textual_outline.expect @@ -0,0 +1,9 @@ +sealed class B {} + +class C extends B { + final int _i; + C(this._i); +} + +f(B b) {} +main() {} diff --git a/pkg/front_end/testcases/patterns/exhaustiveness/issue52041.dart.textual_outline_modelled.expect b/pkg/front_end/testcases/patterns/exhaustiveness/issue52041.dart.textual_outline_modelled.expect new file mode 100644 index 000000000000..81fc74a721d7 --- /dev/null +++ b/pkg/front_end/testcases/patterns/exhaustiveness/issue52041.dart.textual_outline_modelled.expect @@ -0,0 +1,9 @@ +class C extends B { + C(this._i); + final int _i; +} + +f(B b) {} +main() {} + +sealed class B {} diff --git a/pkg/front_end/testcases/patterns/exhaustiveness/issue52041.dart.weak.expect b/pkg/front_end/testcases/patterns/exhaustiveness/issue52041.dart.weak.expect new file mode 100644 index 000000000000..a269b33c3bd2 --- /dev/null +++ b/pkg/front_end/testcases/patterns/exhaustiveness/issue52041.dart.weak.expect @@ -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)); +} diff --git a/pkg/front_end/testcases/patterns/exhaustiveness/issue52041.dart.weak.modular.expect b/pkg/front_end/testcases/patterns/exhaustiveness/issue52041.dart.weak.modular.expect new file mode 100644 index 000000000000..a269b33c3bd2 --- /dev/null +++ b/pkg/front_end/testcases/patterns/exhaustiveness/issue52041.dart.weak.modular.expect @@ -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)); +} diff --git a/pkg/front_end/testcases/patterns/exhaustiveness/issue52041.dart.weak.outline.expect b/pkg/front_end/testcases/patterns/exhaustiveness/issue52041.dart.weak.outline.expect new file mode 100644 index 000000000000..574d3a2412d6 --- /dev/null +++ b/pkg/front_end/testcases/patterns/exhaustiveness/issue52041.dart.weak.outline.expect @@ -0,0 +1,17 @@ +library /*isNonNullableByDefault*/; +import self as self; +import "dart:core" as core; + +abstract sealed class B extends core::Object { + synthetic constructor •() → self::B + ; +} +class C extends self::B { + final field core::int _i; + constructor •(core::int _i) → self::C + ; +} +static method f(self::B b) → dynamic + ; +static method main() → dynamic + ; diff --git a/pkg/front_end/testcases/patterns/exhaustiveness/issue52041.dart.weak.transformed.expect b/pkg/front_end/testcases/patterns/exhaustiveness/issue52041.dart.weak.transformed.expect new file mode 100644 index 000000000000..85fe877baca5 --- /dev/null +++ b/pkg/front_end/testcases/patterns/exhaustiveness/issue52041.dart.weak.transformed.expect @@ -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 core::int #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)); +}