Skip to content

Commit

Permalink
Issue 51137. Don't use the type inferred from the initializer to reso…
Browse files Browse the repository at this point in the history
…lve the initializer.

Bug: #51137
Change-Id: I5979e2531b285d86141d60cd844919456ff17c47
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/281462
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
  • Loading branch information
scheglov authored and Commit Queue committed Feb 8, 2023
1 parent d23c304 commit 0a93b04
Show file tree
Hide file tree
Showing 23 changed files with 1,449 additions and 334 deletions.
2 changes: 1 addition & 1 deletion pkg/analyzer/lib/src/dart/analysis/driver.dart
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ import 'package:analyzer/src/utilities/uri_cache.dart';
/// TODO(scheglov) Clean up the list of implicitly analyzed files.
class AnalysisDriver implements AnalysisDriverGeneric {
/// The version of data format, should be incremented on every format change.
static const int DATA_VERSION = 257;
static const int DATA_VERSION = 258;

/// The number of exception contexts allowed to write. Once this field is
/// zero, we stop writing any new exception contexts in this process.
Expand Down
34 changes: 26 additions & 8 deletions pkg/analyzer/lib/src/dart/element/element.dart
Original file line number Diff line number Diff line change
Expand Up @@ -4952,27 +4952,33 @@ class Modifier implements Comparable<Modifier> {

static const Modifier PROMOTABLE = Modifier('IS_PROMOTABLE', 23);

/// Indicates whether the type of a [PropertyInducingElementImpl] should be
/// used to infer the initializer. We set it to `false` if the type was
/// inferred from the initializer itself.
static const Modifier SHOULD_USE_TYPE_FOR_INITIALIZER_INFERENCE =
Modifier('SHOULD_USE_TYPE_FOR_INITIALIZER_INFERENCE', 24);

/// Indicates that the modifier 'sealed' was applied to the element.
static const Modifier SEALED = Modifier('SEALED', 24);
static const Modifier SEALED = Modifier('SEALED', 25);

/// Indicates that the pseudo-modifier 'set' was applied to the element.
static const Modifier SETTER = Modifier('SETTER', 25);
static const Modifier SETTER = Modifier('SETTER', 26);

/// See [TypeParameterizedElement.isSimplyBounded].
static const Modifier SIMPLY_BOUNDED = Modifier('SIMPLY_BOUNDED', 26);
static const Modifier SIMPLY_BOUNDED = Modifier('SIMPLY_BOUNDED', 27);

/// Indicates that the modifier 'static' was applied to the element.
static const Modifier STATIC = Modifier('STATIC', 27);
static const Modifier STATIC = Modifier('STATIC', 28);

/// Indicates that the element does not appear in the source code but was
/// implicitly created. For example, if a class does not define any
/// constructors, an implicit zero-argument constructor will be created and it
/// will be marked as being synthetic.
static const Modifier SYNTHETIC = Modifier('SYNTHETIC', 28);
static const Modifier SYNTHETIC = Modifier('SYNTHETIC', 29);

/// Indicates that the element was appended to this enclosing element to
/// simulate temporary the effect of applying augmentation.
static const Modifier TEMP_AUGMENTATION = Modifier('TEMP_AUGMENTATION', 29);
static const Modifier TEMP_AUGMENTATION = Modifier('TEMP_AUGMENTATION', 30);

static const List<Modifier> values = [
ABSTRACT,
Expand Down Expand Up @@ -5935,7 +5941,9 @@ abstract class PropertyInducingElementImpl

/// Initialize a newly created synthetic element to have the given [name] and
/// [offset].
PropertyInducingElementImpl(super.name, super.offset);
PropertyInducingElementImpl(super.name, super.offset) {
setModifier(Modifier.SHOULD_USE_TYPE_FOR_INITIALIZER_INFERENCE, true);
}

@override
bool get isConstantEvaluated => true;
Expand All @@ -5960,6 +5968,14 @@ abstract class PropertyInducingElementImpl
}
}

bool get shouldUseTypeForInitializerInference {
return hasModifier(Modifier.SHOULD_USE_TYPE_FOR_INITIALIZER_INFERENCE);
}

set shouldUseTypeForInitializerInference(bool value) {
setModifier(Modifier.SHOULD_USE_TYPE_FOR_INITIALIZER_INFERENCE, value);
}

@override
DartType get type => ElementTypeProvider.current.getFieldType(this);

Expand Down Expand Up @@ -5999,7 +6015,9 @@ abstract class PropertyInducingElementImpl
}

// We must be linking, and the type has not been set yet.
return _type = typeInference!.perform();
_type = typeInference!.perform();
shouldUseTypeForInitializerInference = false;
return _type!;
}

/// Return `true` if this variable needs the setter.
Expand Down
136 changes: 118 additions & 18 deletions pkg/analyzer/lib/src/dart/element/generic_inferrer.dart
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,18 @@

import 'dart:math' as math;

import 'package:analyzer/dart/ast/ast.dart' show AstNode;
import 'package:analyzer/dart/ast/ast.dart'
show
Annotation,
AsExpression,
AstNode,
ConstructorName,
Expression,
InvocationExpression,
SimpleIdentifier;
import 'package:analyzer/dart/element/element.dart';
import 'package:analyzer/dart/element/type.dart';
import 'package:analyzer/error/listener.dart' show ErrorReporter;
import 'package:analyzer/src/dart/element/element.dart';
import 'package:analyzer/src/dart/element/nullability_eliminator.dart';
import 'package:analyzer/src/dart/element/type.dart';
Expand All @@ -15,7 +24,8 @@ import 'package:analyzer/src/dart/element/type_constraint_gatherer.dart';
import 'package:analyzer/src/dart/element/type_provider.dart';
import 'package:analyzer/src/dart/element/type_schema.dart';
import 'package:analyzer/src/dart/element/type_system.dart';
import 'package:analyzer/src/dart/error/inference_error_listener.dart';
import 'package:analyzer/src/error/codes.dart'
show CompileTimeErrorCode, HintCode;
import 'package:meta/meta.dart';

/// Tracks upper and lower type bounds for a set of type parameters.
Expand Down Expand Up @@ -49,13 +59,12 @@ class GenericInferrer {
/// The list of type parameters being inferred.
final List<TypeParameterElement> _typeFormals;

/// The [InferenceErrorListener] to which inference errors should be reported,
/// or `null` if errors shouldn't be reported.
final InferenceErrorListener? _inferenceErrorListener;
/// The [ErrorReporter] to which inference errors should be reported, or
/// `null` if errors shouldn't be reported.
final ErrorReporter? errorReporter;

/// The [AstNode] to which errors should be attached. May be `null` if errors
/// are not being reported (that is, if [_inferenceErrorListener] is also
/// `null`).
/// are not being reported (that is, if [errorReporter] is also `null`).
final AstNode? errorNode;

/// Indicates whether the "generic metadata" feature is enabled. When it is,
Expand Down Expand Up @@ -87,10 +96,12 @@ class GenericInferrer {
final Map<TypeParameterElement, DartType> _typesInferredSoFar = {};

GenericInferrer(this._typeSystem, this._typeFormals,
{InferenceErrorListener? inferenceErrorListener,
{this.errorReporter,
this.errorNode,
required this.genericMetadataIsEnabled})
: _inferenceErrorListener = inferenceErrorListener {
required this.genericMetadataIsEnabled}) {
if (errorReporter != null) {
assert(errorNode != null);
}
_typeParameters.addAll(_typeFormals);
for (var formal in _typeFormals) {
_constraints[formal] = [];
Expand Down Expand Up @@ -208,7 +219,9 @@ class GenericInferrer {
if (!success) {
if (failAtError) return null;
hasErrorReported = true;
_inferenceErrorListener?.addCouldNotInferError(errorNode!,
errorReporter?.reportErrorForNode(
CompileTimeErrorCode.COULD_NOT_INFER,
errorNode!,
[parameter.name, _formatError(parameter, inferred, constraints)]);

// Heuristic: even if we failed, keep the erroneous type.
Expand All @@ -220,12 +233,13 @@ class GenericInferrer {
if (inferred is FunctionType &&
inferred.typeFormals.isNotEmpty &&
!genericMetadataIsEnabled &&
_inferenceErrorListener != null) {
errorReporter != null) {
if (failAtError) return null;
hasErrorReported = true;
var typeFormals = inferred.typeFormals;
var typeFormalsStr = typeFormals.map(_elementStr).join(', ');
_inferenceErrorListener?.addCouldNotInferError(errorNode!, [
errorReporter!.reportErrorForNode(
CompileTimeErrorCode.COULD_NOT_INFER, errorNode!, [
parameter.name,
' Inferred candidate type ${_typeStr(inferred)} has type parameters'
' [$typeFormalsStr], but a function with'
Expand All @@ -235,13 +249,16 @@ class GenericInferrer {

if (UnknownInferredType.isKnown(inferred)) {
knownTypes[parameter] = inferred;
} else if (!hasErrorReported && _typeSystem.strictInference) {
} else if (_typeSystem.strictInference) {
// [typeParam] could not be inferred. A result will still be returned
// by [infer], with [typeParam] filled in as its bounds. This is
// considered a failure of inference, under the "strict-inference"
// mode.
hasErrorReported = true;
_inferenceErrorListener?.reportInferenceFailure(errorNode!);
_reportInferenceFailure(
errorReporter: errorReporter,
errorNode: errorNode,
genericMetadataIsEnabled: genericMetadataIsEnabled,
);
}
}

Expand All @@ -259,7 +276,8 @@ class GenericInferrer {
var typeParamBound = Substitution.fromPairs(_typeFormals, inferredTypes)
.substituteType(typeParam.bound ?? typeProvider.objectType);
// TODO(jmesserly): improve this error message.
_inferenceErrorListener?.addCouldNotInferError(errorNode!, [
errorReporter?.reportErrorForNode(
CompileTimeErrorCode.COULD_NOT_INFER, errorNode!, [
typeParam.name,
"\nRecursive bound cannot be instantiated: '$typeParamBound'."
"\nConsider passing explicit type argument(s) "
Expand All @@ -270,6 +288,8 @@ class GenericInferrer {

if (!hasErrorReported) {
_checkArgumentsNotMatchingBounds(
errorNode: errorNode,
errorReporter: errorReporter,
typeArguments: result,
);
}
Expand All @@ -284,6 +304,8 @@ class GenericInferrer {

/// Check that inferred [typeArguments] satisfy the [typeParameters] bounds.
void _checkArgumentsNotMatchingBounds({
required AstNode? errorNode,
required ErrorReporter? errorReporter,
required List<DartType> typeArguments,
}) {
for (int i = 0; i < _typeFormals.length; i++) {
Expand All @@ -299,7 +321,8 @@ class GenericInferrer {
var substitution = Substitution.fromPairs(_typeFormals, typeArguments);
var bound = substitution.substituteType(rawBound);
if (!_typeSystem.isSubtypeOf(argument, bound)) {
_inferenceErrorListener?.addCouldNotInferError(
errorReporter?.reportErrorForNode(
CompileTimeErrorCode.COULD_NOT_INFER,
errorNode!,
[
parameter.name,
Expand Down Expand Up @@ -524,6 +547,83 @@ class GenericInferrer {
}
}

/// Reports an inference failure on [errorNode] according to its type.
void _reportInferenceFailure({
ErrorReporter? errorReporter,
AstNode? errorNode,
required bool genericMetadataIsEnabled,
}) {
if (errorReporter == null || errorNode == null) {
return;
}
if (errorNode.parent is InvocationExpression &&
errorNode.parent?.parent is AsExpression) {
// Casts via `as` do not play a part in downward inference. We allow an
// exception when inference has "failed" but the return value is
// immediately cast with `as`.
return;
}
if (errorNode is ConstructorName &&
!(errorNode.type.type as InterfaceType).element.hasOptionalTypeArgs) {
String constructorName = errorNode.name == null
? errorNode.type.name.name
: '${errorNode.type}.${errorNode.name}';
errorReporter.reportErrorForNode(
HintCode.INFERENCE_FAILURE_ON_INSTANCE_CREATION,
errorNode,
[constructorName]);
} else if (errorNode is Annotation) {
if (genericMetadataIsEnabled) {
// Only report an error if generic metadata is valid syntax.
var element = errorNode.name.staticElement;
if (element != null && !element.hasOptionalTypeArgs) {
String constructorName = errorNode.constructorName == null
? errorNode.name.name
: '${errorNode.name.name}.${errorNode.constructorName}';
errorReporter.reportErrorForNode(
HintCode.INFERENCE_FAILURE_ON_INSTANCE_CREATION,
errorNode,
[constructorName]);
}
}
} else if (errorNode is SimpleIdentifier) {
var element = errorNode.staticElement;
if (element != null) {
if (element is VariableElement) {
// For variable elements, we check their type and possible alias type.
var type = element.type;
final typeElement = type is InterfaceType ? type.element : null;
if (typeElement != null && typeElement.hasOptionalTypeArgs) {
return;
}
var typeAliasElement = type.alias?.element;
if (typeAliasElement != null &&
typeAliasElement.hasOptionalTypeArgs) {
return;
}
}
if (!element.hasOptionalTypeArgs) {
errorReporter.reportErrorForNode(
HintCode.INFERENCE_FAILURE_ON_FUNCTION_INVOCATION,
errorNode,
[errorNode.name]);
return;
}
}
} else if (errorNode is Expression) {
var type = errorNode.staticType;
if (type != null) {
var typeDisplayString = type.getDisplayString(
withNullability: _typeSystem.isNonNullableByDefault);
errorReporter.reportErrorForNode(
HintCode.INFERENCE_FAILURE_ON_GENERIC_INVOCATION,
errorNode,
[typeDisplayString]);
return;
}
}
}

/// If in a legacy library, return the legacy version of the [type].
/// Otherwise, return the original type.
DartType _toLegacyElementIfOptOut(DartType type) {
Expand Down
19 changes: 2 additions & 17 deletions pkg/analyzer/lib/src/dart/element/type_system.dart
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ import 'package:analyzer/src/dart/element/type_provider.dart';
import 'package:analyzer/src/dart/element/type_schema.dart';
import 'package:analyzer/src/dart/element/type_schema_elimination.dart';
import 'package:analyzer/src/dart/element/well_bounded.dart';
import 'package:analyzer/src/dart/error/inference_error_listener.dart';
import 'package:analyzer/src/utilities/extensions/collection.dart';

/// Fresh type parameters created to unify two lists of type parameters.
Expand Down Expand Up @@ -495,13 +494,7 @@ class TypeSystemImpl implements TypeSystem {
// subtypes (or supertypes) as necessary, and track the constraints that
// are implied by this.
var inferrer = GenericInferrer(this, fnType.typeFormals,
inferenceErrorListener: errorReporter == null
? null
: InferenceErrorReporter(
errorReporter,
isNonNullableByDefault: isNonNullableByDefault,
isGenericMetadataEnabled: genericMetadataIsEnabled,
),
errorReporter: errorReporter,
errorNode: errorNode,
genericMetadataIsEnabled: genericMetadataIsEnabled);
inferrer.constrainGenericFunctionInContext(fnType, contextType);
Expand Down Expand Up @@ -1540,23 +1533,15 @@ class TypeSystemImpl implements TypeSystem {
required DartType declaredReturnType,
required DartType? contextReturnType,
ErrorReporter? errorReporter,
InferenceErrorListener? inferenceErrorListener,
AstNode? errorNode,
required bool genericMetadataIsEnabled,
bool isConst = false}) {
// Create a GenericInferrer that will allow certain type parameters to be
// inferred. It will optimistically assume these type parameters can be
// subtypes (or supertypes) as necessary, and track the constraints that
// are implied by this.
if (errorReporter != null) {
inferenceErrorListener ??= InferenceErrorReporter(
errorReporter,
isNonNullableByDefault: isNonNullableByDefault,
isGenericMetadataEnabled: genericMetadataIsEnabled,
);
}
var inferrer = GenericInferrer(this, typeParameters,
inferenceErrorListener: inferenceErrorListener,
errorReporter: errorReporter,
errorNode: errorNode,
genericMetadataIsEnabled: genericMetadataIsEnabled);

Expand Down
Loading

0 comments on commit 0a93b04

Please sign in to comment.