Skip to content

Commit

Permalink
[cfe] Disallow super-bounded aliased constructor invocations
Browse files Browse the repository at this point in the history
This CL also threads through the inferredness of type arguments for
the factories, so that the error that is checked twice isn't reported
twice because the arguments are recognized as inferred in one place
and as explicit in the other.

Closes #42446.

Bug: #42446
Change-Id: I8cc158b6d8070d34c1489386962fa1f5e2592719
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/184270
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Dmitry Stefantsov <dmitryas@google.com>
  • Loading branch information
Dmitry Stefantsov authored and commit-bot@chromium.org committed Feb 15, 2021
1 parent 774b80f commit b994d51
Show file tree
Hide file tree
Showing 9 changed files with 208 additions and 32 deletions.
12 changes: 9 additions & 3 deletions pkg/front_end/lib/src/fasta/kernel/body_builder.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1226,7 +1226,9 @@ class BodyBuilder extends ScopeListener<JumpTarget>
replacementNode = buildStaticInvocation(
resolvedTarget,
forest.createArguments(noLocation, arguments.positional,
types: arguments.types, named: arguments.named),
types: arguments.types,
named: arguments.named,
hasExplicitTypeArguments: hasExplicitTypeArguments(arguments)),
constness:
isConst ? Constness.explicitConst : Constness.explicitNew,
charOffset: fileOffset);
Expand Down Expand Up @@ -1298,7 +1300,10 @@ class BodyBuilder extends ScopeListener<JumpTarget>
}
Arguments invocationArguments = forest.createArguments(
noLocation, invocation.arguments.positional,
types: invocationTypeArguments, named: invocation.arguments.named);
types: invocationTypeArguments,
named: invocation.arguments.named,
hasExplicitTypeArguments:
hasExplicitTypeArguments(invocation.arguments));
invocation.replaceWith(_resolveRedirectingFactoryTarget(invocation.target,
invocationArguments, invocation.fileOffset, invocation.isConst));
}
Expand Down Expand Up @@ -4162,7 +4167,8 @@ class BodyBuilder extends ScopeListener<JumpTarget>
isConst: isConst)
..fileOffset = charOffset;
libraryBuilder.checkBoundsInFactoryInvocation(
node, typeEnvironment, uri);
node, typeEnvironment, uri,
inferred: !hasExplicitTypeArguments(arguments));
} else {
node = new TypeAliasedFactoryInvocationJudgment(
typeAliasBuilder, target, arguments,
Expand Down
15 changes: 12 additions & 3 deletions pkg/front_end/lib/src/fasta/kernel/forest.dart
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,18 @@ class Forest {
const Forest();

Arguments createArguments(int fileOffset, List<Expression> positional,
{List<DartType> types, List<NamedExpression> named}) {
return new ArgumentsImpl(positional, types: types, named: named)
..fileOffset = fileOffset ?? TreeNode.noOffset;
{List<DartType> types,
List<NamedExpression> named,
bool hasExplicitTypeArguments = true}) {
if (!hasExplicitTypeArguments) {
ArgumentsImpl arguments =
new ArgumentsImpl(positional, types: <DartType>[], named: named);
arguments.types.addAll(types);
return arguments;
} else {
return new ArgumentsImpl(positional, types: types, named: named)
..fileOffset = fileOffset ?? TreeNode.noOffset;
}
}

Arguments createArgumentsForExtensionMethod(
Expand Down
12 changes: 5 additions & 7 deletions pkg/front_end/lib/src/fasta/kernel/inference_visitor.dart
Original file line number Diff line number Diff line change
Expand Up @@ -404,8 +404,7 @@ class InferenceVisitor
ExpressionInferenceResult visitConstructorInvocation(
ConstructorInvocation node, DartType typeContext) {
inferrer.inferConstructorParameterTypes(node.target);
bool hasExplicitTypeArguments =
getExplicitTypeArguments(node.arguments) != null;
bool hadExplicitTypeArguments = hasExplicitTypeArguments(node.arguments);
FunctionType functionType = replaceReturnType(
node.target.function
.computeThisFunctionType(inferrer.library.nonNullable),
Expand All @@ -415,7 +414,7 @@ class InferenceVisitor
isConst: node.isConst, staticTarget: node.target);
if (!inferrer.isTopLevel) {
SourceLibraryBuilder library = inferrer.library;
if (!hasExplicitTypeArguments) {
if (!hadExplicitTypeArguments) {
library.checkBoundsInConstructorInvocation(
node, inferrer.typeSchemaEnvironment, inferrer.helper.uri,
inferred: true);
Expand Down Expand Up @@ -692,8 +691,7 @@ class InferenceVisitor

ExpressionInferenceResult visitFactoryConstructorInvocationJudgment(
FactoryConstructorInvocationJudgment node, DartType typeContext) {
bool hadExplicitTypeArguments =
getExplicitTypeArguments(node.arguments) != null;
bool hadExplicitTypeArguments = hasExplicitTypeArguments(node.arguments);

FunctionType functionType = replaceReturnType(
node.target.function
Expand Down Expand Up @@ -740,7 +738,7 @@ class InferenceVisitor
SourceLibraryBuilder library = inferrer.library;
library.checkBoundsInType(result.inferredType,
inferrer.typeSchemaEnvironment, inferrer.helper.uri, node.fileOffset,
inferred: true);
inferred: true, allowSuperBounded: false);
if (inferrer.isNonNullableByDefault) {
if (node.target == inferrer.coreTypes.listDefaultConstructor) {
resultNode = inferrer.helper.wrapInProblem(node,
Expand Down Expand Up @@ -768,7 +766,7 @@ class InferenceVisitor
SourceLibraryBuilder library = inferrer.library;
library.checkBoundsInType(result.inferredType,
inferrer.typeSchemaEnvironment, inferrer.helper.uri, node.fileOffset,
inferred: true);
inferred: true, allowSuperBounded: false);
if (inferrer.isNonNullableByDefault) {
if (node.target == inferrer.coreTypes.listDefaultConstructor) {
resultNode = inferrer.helper.wrapInProblem(node,
Expand Down
4 changes: 4 additions & 0 deletions pkg/front_end/lib/src/fasta/kernel/internal_ast.dart
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,10 @@ List<DartType> getExplicitTypeArguments(Arguments arguments) {
}
}

bool hasExplicitTypeArguments(Arguments arguments) {
return getExplicitTypeArguments(arguments) != null;
}

/// Information associated with a class during type inference.
class ClassInferenceInfo {
/// The builder associated with this class.
Expand Down
20 changes: 20 additions & 0 deletions pkg/front_end/testcases/nonfunction_type_aliases/issue42446.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// Copyright (c) 2021, 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.

class A<X extends A<X>> {}
typedef B<X extends A<X>> = A<X>;

class A2<X extends A2<X>> {
factory A2() => throw 42;
}
typedef B2<X extends A2<X>> = A2<X>;

foo() {
B(); // Error.
A(); // Error.
B2(); // Error.
A2(); // Error.
}

main() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
class A<X extends A<X>> {}
typedef B<X extends A<X>> = A<X>;
class A2<X extends A2<X>> {
factory A2() => throw 42;
}
typedef B2<X extends A2<X>> = A2<X>;
foo() {}
main() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
library /*isNonNullableByDefault*/;
//
// Problems in library:
//
// pkg/front_end/testcases/nonfunction_type_aliases/issue42446.dart:14:3: Error: Inferred type argument 'A<Object?>' doesn't conform to the bound 'A<X>' of the type variable 'X' on 'A'.
// - 'A' is from 'pkg/front_end/testcases/nonfunction_type_aliases/issue42446.dart'.
// - 'Object' is from 'dart:core'.
// Try specifying type arguments explicitly so that they conform to the bounds.
// B(); // Error.
// ^
// pkg/front_end/testcases/nonfunction_type_aliases/issue42446.dart:5:9: Context: This is the type variable whose bound isn't conformed to.
// class A<X extends A<X>> {}
// ^
//
// pkg/front_end/testcases/nonfunction_type_aliases/issue42446.dart:15:3: Error: Inferred type argument 'A<Object?>' doesn't conform to the bound 'A<X>' of the type variable 'X' on 'A'.
// - 'A' is from 'pkg/front_end/testcases/nonfunction_type_aliases/issue42446.dart'.
// - 'Object' is from 'dart:core'.
// Try specifying type arguments explicitly so that they conform to the bounds.
// A(); // Error.
// ^
// pkg/front_end/testcases/nonfunction_type_aliases/issue42446.dart:5:9: Context: This is the type variable whose bound isn't conformed to.
// class A<X extends A<X>> {}
// ^
//
// pkg/front_end/testcases/nonfunction_type_aliases/issue42446.dart:16:3: Error: Inferred type argument 'A2<Object?>' doesn't conform to the bound 'A2<X>' of the type variable 'X' on 'A2'.
// - 'A2' is from 'pkg/front_end/testcases/nonfunction_type_aliases/issue42446.dart'.
// - 'Object' is from 'dart:core'.
// Try specifying type arguments explicitly so that they conform to the bounds.
// B2(); // Error.
// ^
// pkg/front_end/testcases/nonfunction_type_aliases/issue42446.dart:8:10: Context: This is the type variable whose bound isn't conformed to.
// class A2<X extends A2<X>> {
// ^
//
// pkg/front_end/testcases/nonfunction_type_aliases/issue42446.dart:17:3: Error: Inferred type argument 'A2<Object?>' doesn't conform to the bound 'A2<X>' of the type variable 'X' on 'A2'.
// - 'A2' is from 'pkg/front_end/testcases/nonfunction_type_aliases/issue42446.dart'.
// - 'Object' is from 'dart:core'.
// Try specifying type arguments explicitly so that they conform to the bounds.
// A2(); // Error.
// ^
// pkg/front_end/testcases/nonfunction_type_aliases/issue42446.dart:8:10: Context: This is the type variable whose bound isn't conformed to.
// class A2<X extends A2<X>> {
// ^
//
import self as self;
import "dart:core" as core;

typedef B<X extends self::A<X> = self::A<dynamic>> = self::A<X>;
typedef B2<X extends self::A2<X> = self::A2<dynamic>> = self::A2<X>;
class A<X extends self::A<self::A::X> = self::A<dynamic>> extends core::Object {
synthetic constructor •() → self::A<self::A::X>
: super core::Object::•()
;
}
class A2<X extends self::A2<self::A2::X> = self::A2<dynamic>> extends core::Object {
static factory •<X extends self::A2<self::A2::•::X> = self::A2<dynamic>>() → self::A2<self::A2::•::X>
return throw 42;
}
static method foo() → dynamic {
new self::A::•<self::A<core::Object?>>();
new self::A::•<self::A<core::Object?>>();
self::A2::•<self::A2<core::Object?>>();
self::A2::•<self::A2<core::Object?>>();
}
static method main() → dynamic {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
library /*isNonNullableByDefault*/;
//
// Problems in library:
//
// pkg/front_end/testcases/nonfunction_type_aliases/issue42446.dart:14:3: Error: Inferred type argument 'A<Object?>' doesn't conform to the bound 'A<X>' of the type variable 'X' on 'A'.
// - 'A' is from 'pkg/front_end/testcases/nonfunction_type_aliases/issue42446.dart'.
// - 'Object' is from 'dart:core'.
// Try specifying type arguments explicitly so that they conform to the bounds.
// B(); // Error.
// ^
// pkg/front_end/testcases/nonfunction_type_aliases/issue42446.dart:5:9: Context: This is the type variable whose bound isn't conformed to.
// class A<X extends A<X>> {}
// ^
//
// pkg/front_end/testcases/nonfunction_type_aliases/issue42446.dart:15:3: Error: Inferred type argument 'A<Object?>' doesn't conform to the bound 'A<X>' of the type variable 'X' on 'A'.
// - 'A' is from 'pkg/front_end/testcases/nonfunction_type_aliases/issue42446.dart'.
// - 'Object' is from 'dart:core'.
// Try specifying type arguments explicitly so that they conform to the bounds.
// A(); // Error.
// ^
// pkg/front_end/testcases/nonfunction_type_aliases/issue42446.dart:5:9: Context: This is the type variable whose bound isn't conformed to.
// class A<X extends A<X>> {}
// ^
//
// pkg/front_end/testcases/nonfunction_type_aliases/issue42446.dart:16:3: Error: Inferred type argument 'A2<Object?>' doesn't conform to the bound 'A2<X>' of the type variable 'X' on 'A2'.
// - 'A2' is from 'pkg/front_end/testcases/nonfunction_type_aliases/issue42446.dart'.
// - 'Object' is from 'dart:core'.
// Try specifying type arguments explicitly so that they conform to the bounds.
// B2(); // Error.
// ^
// pkg/front_end/testcases/nonfunction_type_aliases/issue42446.dart:8:10: Context: This is the type variable whose bound isn't conformed to.
// class A2<X extends A2<X>> {
// ^
//
// pkg/front_end/testcases/nonfunction_type_aliases/issue42446.dart:17:3: Error: Inferred type argument 'A2<Object?>' doesn't conform to the bound 'A2<X>' of the type variable 'X' on 'A2'.
// - 'A2' is from 'pkg/front_end/testcases/nonfunction_type_aliases/issue42446.dart'.
// - 'Object' is from 'dart:core'.
// Try specifying type arguments explicitly so that they conform to the bounds.
// A2(); // Error.
// ^
// pkg/front_end/testcases/nonfunction_type_aliases/issue42446.dart:8:10: Context: This is the type variable whose bound isn't conformed to.
// class A2<X extends A2<X>> {
// ^
//
import self as self;
import "dart:core" as core;

typedef B<X extends self::A<X> = self::A<dynamic>> = self::A<X>;
typedef B2<X extends self::A2<X> = self::A2<dynamic>> = self::A2<X>;
class A<X extends self::A<self::A::X> = self::A<dynamic>> extends core::Object {
synthetic constructor •() → self::A<self::A::X>
: super core::Object::•()
;
}
class A2<X extends self::A2<self::A2::X> = self::A2<dynamic>> extends core::Object {
static factory •<X extends self::A2<self::A2::•::X> = self::A2<dynamic>>() → self::A2<self::A2::•::X>
return throw 42;
}
static method foo() → dynamic {
new self::A::•<self::A<core::Object?>>();
new self::A::•<self::A<core::Object?>>();
self::A2::•<self::A2<core::Object?>>();
self::A2::•<self::A2<core::Object?>>();
}
static method main() → dynamic {}
39 changes: 20 additions & 19 deletions pkg/front_end/testcases/textual_outline.status
Original file line number Diff line number Diff line change
Expand Up @@ -20,21 +20,12 @@ regress/issue_39035.crash: EmptyOutput
regress/issue_39091_2: EmptyOutput
regress/utf_16_le_content.crash: EmptyOutput


extensions/extension_constructor: FormatterCrash
extensions/extension_field_with_type_parameter_usage: FormatterCrash
extensions/issue38600: FormatterCrash
extensions/issue38712: FormatterCrash
extensions/issue38745: FormatterCrash
general_nnbd_opt_out/annotation_eof: FormatterCrash
general_nnbd_opt_out/bad_setter_abstract: FormatterCrash
general_nnbd_opt_out/bug31124: FormatterCrash
general_nnbd_opt_out/clone_function_type: FormatterCrash
general_nnbd_opt_out/constructor_initializer_invalid: FormatterCrash
general_nnbd_opt_out/duplicated_declarations: FormatterCrash
general_nnbd_opt_out/function_type_default_value: FormatterCrash
general_nnbd_opt_out/incomplete_field_formal_parameter: FormatterCrash
general_nnbd_opt_out/many_errors: FormatterCrash
general_nnbd_opt_out/var_as_type_name: FormatterCrash
general/annotation_eof: FormatterCrash
general/bad_setter_abstract: FormatterCrash
general/bug31124: FormatterCrash
Expand Down Expand Up @@ -62,21 +53,31 @@ general/error_recovery/issue_39958_01: FormatterCrash
general/error_recovery/issue_43090.crash: FormatterCrash
general/function_type_default_value: FormatterCrash
general/incomplete_field_formal_parameter: FormatterCrash
general/invalid_operator: FormatterCrash
general/invalid_operator2: FormatterCrash
general/invalid_operator: FormatterCrash
general/issue42997: FormatterCrash
general/issue43363: FormatterCrash
general/many_errors: FormatterCrash
general/null_safety_invalid_experiment_and_language_version: FormatterCrash
general/type_parameters_on_void: FormatterCrash
general/var_as_type_name: FormatterCrash
general_nnbd_opt_out/annotation_eof: FormatterCrash
general_nnbd_opt_out/bad_setter_abstract: FormatterCrash
general_nnbd_opt_out/bug31124: FormatterCrash
general_nnbd_opt_out/clone_function_type: FormatterCrash
general_nnbd_opt_out/constructor_initializer_invalid: FormatterCrash
general_nnbd_opt_out/duplicated_declarations: FormatterCrash
general_nnbd_opt_out/function_type_default_value: FormatterCrash
general_nnbd_opt_out/incomplete_field_formal_parameter: FormatterCrash
general_nnbd_opt_out/many_errors: FormatterCrash
general_nnbd_opt_out/var_as_type_name: FormatterCrash
inference/unsafe_block_closure_inference_function_call_explicit_dynamic_param_via_expr1: FormatterCrash
inference/unsafe_block_closure_inference_function_call_explicit_type_param_via_expr1: FormatterCrash
late_lowering/covariant_late_field: FormatterCrash
late_lowering/getter_vs_setter_type: FormatterCrash
late_lowering/infer_late_field_type: FormatterCrash
late_lowering/initializer_rewrite_from_opt_out: FormatterCrash
late_lowering/initializer_rewrite: FormatterCrash
late_lowering/initializer_rewrite_from_opt_out: FormatterCrash
late_lowering/instance_field_with_initializer: FormatterCrash
late_lowering/instance_field_without_initializer: FormatterCrash
late_lowering/instance_final_field_without_initializer: FormatterCrash
Expand All @@ -103,19 +104,16 @@ late_lowering/late_nullable_field_with_initializer: FormatterCrash
late_lowering/late_nullable_field_without_initializer: FormatterCrash
late_lowering/late_override: FormatterCrash
late_lowering/later: FormatterCrash
late_lowering/override_getter_setter: FormatterCrash
late_lowering/override: FormatterCrash
late_lowering/override_getter_setter: FormatterCrash
late_lowering/skip_late_final_uninitialized_instance_fields/main: FormatterCrash
late_lowering/uninitialized_non_nullable_late_fields: FormatterCrash
nnbd_mixed/inheritance_from_opt_in: FormatterCrash
nnbd_mixed/issue41597: FormatterCrash
nnbd_mixed/null_safety_invalid_language_version: FormatterCrash
nnbd/abstract_field_errors: FormatterCrash
nnbd/covariant_late_field: FormatterCrash
nnbd/duplicates_instance_extension: FormatterCrash
nnbd/duplicates_instance: FormatterCrash
nnbd/duplicates_static_extension: FormatterCrash
nnbd/duplicates_instance_extension: FormatterCrash
nnbd/duplicates_static: FormatterCrash
nnbd/duplicates_static_extension: FormatterCrash
nnbd/field_vs_setter: FormatterCrash
nnbd/forbidden_supers: FormatterCrash
nnbd/getter_vs_setter_type_late: FormatterCrash
Expand All @@ -132,7 +130,11 @@ nnbd/nonfield_vs_setter: FormatterCrash
nnbd/opt_out: FormatterCrash
nnbd/potentially_non_nullable_field: FormatterCrash
nnbd/uninitialized_non_nullable_late_fields: FormatterCrash
nnbd_mixed/inheritance_from_opt_in: FormatterCrash
nnbd_mixed/issue41597: FormatterCrash
nnbd_mixed/null_safety_invalid_language_version: FormatterCrash
nonfunction_type_aliases/issue41501: FormatterCrash
nonfunction_type_aliases/issue42446: FormatterCrash
rasta/bad_redirection: FormatterCrash
rasta/issue_000032: FormatterCrash
rasta/issue_000034: FormatterCrash
Expand Down Expand Up @@ -172,4 +174,3 @@ variance/class_type_parameter_modifier: FormatterCrash
variance/generic_covariance_sound_variance: FormatterCrash
variance/mixin_type_parameter_modifier: FormatterCrash
variance/unconstrained_inference: FormatterCrash

0 comments on commit b994d51

Please sign in to comment.