Skip to content

Commit

Permalink
Remove TrustTypeAnnotations
Browse files Browse the repository at this point in the history
Change-Id: I7404d10c30642f5282d079e36cb4a20ee2066060
Reviewed-on: https://dart-review.googlesource.com/c/87401
Reviewed-by: Sigmund Cherem <sigmund@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Johnni Winther <johnniwinther@google.com>
  • Loading branch information
johnniwinther authored and commit-bot@chromium.org committed Dec 18, 2018
1 parent ecccc0d commit 6d4d3c6
Show file tree
Hide file tree
Showing 12 changed files with 7 additions and 160 deletions.
15 changes: 1 addition & 14 deletions pkg/compiler/lib/src/common_elements.dart
Original file line number Diff line number Diff line change
Expand Up @@ -473,8 +473,6 @@ abstract class CommonElements {

ClassEntity get expectNoInlineClass;

ClassEntity get expectTrustTypeAnnotationsClass;

ClassEntity get expectAssumeDynamicClass;

/// Returns `true` if [member] is a "foreign helper", that is, a member whose
Expand Down Expand Up @@ -1666,7 +1664,6 @@ class CommonElementsImpl

bool _expectAnnotationChecked = false;
ClassEntity _expectNoInlineClass;
ClassEntity _expectTrustTypeAnnotationsClass;
ClassEntity _expectAssumeDynamicClass;

void _ensureExpectAnnotations() {
Expand All @@ -1675,15 +1672,10 @@ class CommonElementsImpl
LibraryEntity library = _env.lookupLibrary(PACKAGE_EXPECT);
if (library != null) {
_expectNoInlineClass = _env.lookupClass(library, 'NoInline');
_expectTrustTypeAnnotationsClass =
_env.lookupClass(library, 'TrustTypeAnnotations');
_expectAssumeDynamicClass = _env.lookupClass(library, 'AssumeDynamic');
if (_expectNoInlineClass == null ||
_expectTrustTypeAnnotationsClass == null ||
_expectAssumeDynamicClass == null) {
if (_expectNoInlineClass == null || _expectAssumeDynamicClass == null) {
// This is not the package you're looking for.
_expectNoInlineClass = null;
_expectTrustTypeAnnotationsClass = null;
_expectAssumeDynamicClass = null;
}
}
Expand All @@ -1695,11 +1687,6 @@ class CommonElementsImpl
return _expectNoInlineClass;
}

ClassEntity get expectTrustTypeAnnotationsClass {
_ensureExpectAnnotations();
return _expectTrustTypeAnnotationsClass;
}

ClassEntity get expectAssumeDynamicClass {
_ensureExpectAnnotations();
return _expectAssumeDynamicClass;
Expand Down
1 change: 0 additions & 1 deletion pkg/compiler/lib/src/dart2js.dart
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,6 @@ Future<api.CompilationResult> compile(List<String> argv,

void setTrustTypeAnnotations(String argument) {
trustTypeAnnotations = true;
passThrough(argument);
}

void setCheckedMode(String argument) {
Expand Down
10 changes: 0 additions & 10 deletions pkg/compiler/lib/src/inferrer/inferrer_engine.dart
Original file line number Diff line number Diff line change
Expand Up @@ -242,10 +242,6 @@ abstract class InferrerEngine {
bool canFunctionParametersBeUsedForGlobalOptimizations(
FunctionEntity function);

/// Returns `true` if parameter and returns types should be trusted for
/// [member].
bool trustTypeAnnotations(MemberEntity member);

/// Returns `true` if inference of parameter types is disabled for [member].
bool assumeDynamic(MemberEntity member);
}
Expand Down Expand Up @@ -1246,12 +1242,6 @@ class InferrerEngineImpl extends InferrerEngine {
return !closedWorld.backendUsage.isFunctionUsedByBackend(function);
}

@override
bool trustTypeAnnotations(MemberEntity member) {
return closedWorld.annotationsData.trustTypeAnnotationsMembers
.contains(member);
}

@override
bool assumeDynamic(MemberEntity member) {
return closedWorld.annotationsData.assumeDynamicMembers.contains(member);
Expand Down
6 changes: 2 additions & 4 deletions pkg/compiler/lib/src/inferrer/type_graph_nodes.dart
Original file line number Diff line number Diff line change
Expand Up @@ -478,8 +478,7 @@ abstract class MemberTypeInformation extends ElementTypeInformation
AbstractValue potentiallyNarrowType(
AbstractValue mask, InferrerEngine inferrer) {
if (inferrer.options.assignmentCheckPolicy.isTrusted ||
inferrer.options.assignmentCheckPolicy.isEmitted ||
inferrer.trustTypeAnnotations(_member)) {
inferrer.options.assignmentCheckPolicy.isEmitted) {
return _potentiallyNarrowType(mask, inferrer);
}
return mask;
Expand Down Expand Up @@ -792,8 +791,7 @@ class ParameterTypeInformation extends ElementTypeInformation {

AbstractValue potentiallyNarrowType(
AbstractValue mask, InferrerEngine inferrer) {
if (inferrer.options.parameterCheckPolicy.isTrusted ||
inferrer.trustTypeAnnotations(_method)) {
if (inferrer.options.parameterCheckPolicy.isTrusted) {
// In checked or strong mode we don't trust the types of the arguments
// passed to a parameter. The means that the checking of a parameter is
// based on the actual arguments.
Expand Down
28 changes: 0 additions & 28 deletions pkg/compiler/lib/src/js_backend/annotations.dart
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,6 @@ import '../elements/entities.dart';
import '../kernel/dart2js_target.dart';
import '../serialization/serialization.dart';

/// Returns `true` if parameter and returns types should be trusted for
/// [element].
bool _trustTypeAnnotations(KElementEnvironment elementEnvironment,
KCommonElements commonElements, MemberEntity element) {
return _hasAnnotation(elementEnvironment, element,
commonElements.expectTrustTypeAnnotationsClass);
}

/// Returns `true` if inference of parameter types is disabled for [element].
bool _assumeDynamic(KElementEnvironment elementEnvironment,
KCommonElements commonElements, MemberEntity element) {
Expand Down Expand Up @@ -63,11 +55,6 @@ Set<PragmaAnnotation> processMemberAnnotations(
bool hasTryInline = false;
bool disableFinal = false;

if (_trustTypeAnnotations(elementEnvironment, commonElements, element)) {
values.add(PragmaAnnotation.trustTypeAnnotations);
annotationsDataBuilder.registerTrustTypeAnnotations(element);
}

if (_assumeDynamic(elementEnvironment, commonElements, element)) {
values.add(PragmaAnnotation.assumeDynamic);
annotationsDataBuilder.registerAssumeDynamic(element);
Expand Down Expand Up @@ -222,9 +209,6 @@ abstract class AnnotationsData {
/// Functions with a `@NoSideEffects()` annotation.
Iterable<FunctionEntity> get sideEffectFreeFunctions;

/// Members with a `@TrustTypeAnnotations()` annotation.
Iterable<MemberEntity> get trustTypeAnnotationsMembers;

/// Members with a `@AssumeDynamic()` annotation.
Iterable<MemberEntity> get assumeDynamicMembers;
}
Expand All @@ -239,7 +223,6 @@ class AnnotationsDataImpl implements AnnotationsData {
final Iterable<FunctionEntity> disableFinalFunctions;
final Iterable<FunctionEntity> cannotThrowFunctions;
final Iterable<FunctionEntity> sideEffectFreeFunctions;
final Iterable<MemberEntity> trustTypeAnnotationsMembers;
final Iterable<MemberEntity> assumeDynamicMembers;

AnnotationsDataImpl(
Expand All @@ -248,7 +231,6 @@ class AnnotationsDataImpl implements AnnotationsData {
this.disableFinalFunctions,
this.cannotThrowFunctions,
this.sideEffectFreeFunctions,
this.trustTypeAnnotationsMembers,
this.assumeDynamicMembers);

factory AnnotationsDataImpl.readFromDataSource(DataSource source) {
Expand All @@ -268,9 +250,6 @@ class AnnotationsDataImpl implements AnnotationsData {
Iterable<FunctionEntity> sideEffectFreeFunctions =
source.readMembers<FunctionEntity>(emptyAsNull: true) ??
const <FunctionEntity>[];
Iterable<MemberEntity> trustTypeAnnotationsMembers =
source.readMembers<MemberEntity>(emptyAsNull: true) ??
const <MemberEntity>[];
Iterable<MemberEntity> assumeDynamicMembers =
source.readMembers<MemberEntity>(emptyAsNull: true) ??
const <MemberEntity>[];
Expand All @@ -281,7 +260,6 @@ class AnnotationsDataImpl implements AnnotationsData {
disableFinalFunctions,
cannotThrowFunctions,
sideEffectFreeFunctions,
trustTypeAnnotationsMembers,
assumeDynamicMembers);
}

Expand All @@ -292,7 +270,6 @@ class AnnotationsDataImpl implements AnnotationsData {
sink.writeMembers(disableFinalFunctions);
sink.writeMembers(cannotThrowFunctions);
sink.writeMembers(sideEffectFreeFunctions);
sink.writeMembers(trustTypeAnnotationsMembers);
sink.writeMembers(assumeDynamicMembers);
sink.end(tag);
}
Expand Down Expand Up @@ -332,11 +309,6 @@ class AnnotationsDataBuilder implements AnnotationsData {
_sideEffectFreeFunctions.add(function);
}

void registerTrustTypeAnnotations(MemberEntity member) {
_trustTypeAnnotationsMembers ??= <MemberEntity>[];
_trustTypeAnnotationsMembers.add(member);
}

void registerAssumeDynamic(MemberEntity member) {
_assumeDynamicMembers ??= <MemberEntity>[];
_assumeDynamicMembers.add(member);
Expand Down
2 changes: 0 additions & 2 deletions pkg/compiler/lib/src/js_model/js_world_builder.dart
Original file line number Diff line number Diff line change
Expand Up @@ -203,8 +203,6 @@ class JsClosedWorldBuilder {
closedWorld.annotationsData.cannotThrowFunctions),
map.toBackendFunctionSet(
closedWorld.annotationsData.sideEffectFreeFunctions),
map.toBackendMemberSet(
closedWorld.annotationsData.trustTypeAnnotationsMembers),
map.toBackendMemberSet(
closedWorld.annotationsData.assumeDynamicMembers));

Expand Down
14 changes: 0 additions & 14 deletions pkg/expect/lib/expect.dart
Original file line number Diff line number Diff line change
Expand Up @@ -670,20 +670,6 @@ class NoInline {
const NoInline();
}

/// Annotation class for testing of dart2js. Use this as metadata on method
/// declarations to make the type inferrer trust the parameter and return types,
/// effectively asserting the runtime values will (at least) be subtypes of the
/// annotated types.
///
/// While the actually inferred type is guaranteed to be a subtype of the
/// annotation, it often is more precise. In particular, if a method is only
/// called with `null`, the inferrer will still infer null. To ensure that
/// the annotated type is also the inferred type, additionally use
/// [AssumeDynamic].
class TrustTypeAnnotations {
const TrustTypeAnnotations();
}

/// Annotation class for testing of dart2js. Use this as metadata on method
/// declarations to disable closed world assumptions on parameters, effectively
/// assuming that the runtime arguments could be any value. Note that the
Expand Down
43 changes: 1 addition & 42 deletions tests/compiler/dart2js/codegen/expect_annotations_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -21,28 +21,13 @@ int method(String arg) => arg.length;
@AssumeDynamic()
int methodAssumeDynamic(String arg) => arg.length;
@TrustTypeAnnotations()
int methodTrustTypeAnnotations(String arg) => arg.length;
@NoInline()
int methodNoInline(String arg) => arg.length;
@NoInline() @TrustTypeAnnotations()
int methodNoInlineTrustTypeAnnotations(String arg) => arg.length;
@AssumeDynamic() @TrustTypeAnnotations()
int methodAssumeDynamicTrustTypeAnnotations(String arg) => arg.length;
void main(List<String> args) {
print(method(args[0]));
print(methodAssumeDynamic('foo'));
print(methodTrustTypeAnnotations(42 as dynamic));
print(methodTrustTypeAnnotations("fourtyTwo"));
print(methodNoInline('bar'));
print(methodNoInlineTrustTypeAnnotations(42 as dynamic));
print(methodNoInlineTrustTypeAnnotations("fourtyTwo"));
print(methodAssumeDynamicTrustTypeAnnotations(null));
}
"""
};
Expand All @@ -61,8 +46,6 @@ runTest() async {
Expect.isFalse(compiler.compilationFailed, 'Unsuccessful compilation');
Expect.isNotNull(closedWorld.commonElements.expectNoInlineClass,
'NoInlineClass is unresolved.');
Expect.isNotNull(closedWorld.commonElements.expectTrustTypeAnnotationsClass,
'TrustTypeAnnotations is unresolved.');
Expect.isNotNull(closedWorld.commonElements.expectAssumeDynamicClass,
'AssumeDynamicClass is unresolved.');

Expand All @@ -83,7 +66,6 @@ runTest() async {

void test(String name,
{bool expectNoInline: false,
bool expectTrustTypeAnnotations: false,
TypeMask expectedParameterType: null,
TypeMask expectedReturnType: null,
bool expectAssumeDynamic: false}) {
Expand All @@ -95,42 +77,19 @@ runTest() async {
expectNoInline,
closedWorld.annotationsData.nonInlinableFunctions.contains(method),
"Unexpected annotation of @NoInline() on '$method'.");
Expect.equals(
expectTrustTypeAnnotations,
closedWorld.annotationsData.trustTypeAnnotationsMembers
.contains(method),
"Unexpected annotation of @TrustTypeAnnotations() on '$method'.");
Expect.equals(
expectAssumeDynamic,
closedWorld.annotationsData.assumeDynamicMembers.contains(method),
"Unexpected annotation of @AssumeDynamic() on '$method'.");
GlobalTypeInferenceResults results =
compiler.globalInference.resultsForTesting;
if (expectTrustTypeAnnotations && expectedParameterType != null) {
testTypeMatch(method, expectedParameterType, expectedReturnType, results);
} else if (expectAssumeDynamic) {
if (expectAssumeDynamic) {
testTypeMatch(
method, closedWorld.abstractValueDomain.dynamicType, null, results);
}
}

TypeMask jsStringType = closedWorld.abstractValueDomain.stringType;
TypeMask jsIntType = closedWorld.abstractValueDomain.intType;
TypeMask coreStringType =
new TypeMask.subtype(closedWorld.commonElements.stringClass, closedWorld);

test('method');
test('methodAssumeDynamic', expectAssumeDynamic: true);
test('methodTrustTypeAnnotations',
expectTrustTypeAnnotations: true, expectedParameterType: jsStringType);
test('methodNoInline', expectNoInline: true);
test('methodNoInlineTrustTypeAnnotations',
expectNoInline: true,
expectTrustTypeAnnotations: true,
expectedParameterType: jsStringType,
expectedReturnType: jsIntType);
test('methodAssumeDynamicTrustTypeAnnotations',
expectAssumeDynamic: true,
expectTrustTypeAnnotations: true,
expectedParameterType: coreStringType);
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

import 'package:expect/expect.dart';
import "package:async_helper/async_helper.dart";
import 'package:compiler/src/commandline_options.dart';
import '../helpers/memory_compiler.dart';

const MEMORY_SOURCE_FILES = const {
Expand All @@ -26,9 +25,7 @@ main (x, y) {

main() {
runTest() async {
var options = [Flags.trustTypeAnnotations];
var result = await runCompiler(
memorySourceFiles: MEMORY_SOURCE_FILES, options: options);
var result = await runCompiler(memorySourceFiles: MEMORY_SOURCE_FILES);
var compiler = result.compiler;
var element =
compiler.backendClosedWorldForTesting.elementEnvironment.mainFunction;
Expand Down
6 changes: 0 additions & 6 deletions tests/compiler/dart2js/equivalence/show_helper.dart
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@ ArgParser createArgParser() {
argParser.addFlag('strong', negatable: false, defaultsTo: false);
argParser.addFlag('omit-implicit-checks',
negatable: false, defaultsTo: false);
argParser.addFlag('trust-type-annotations',
negatable: false, defaultsTo: false);
return argParser;
}

Expand All @@ -34,7 +32,6 @@ show(ArgResults argResults, DataComputer dataComputer,
}
bool verbose = argResults['verbose'];
bool omitImplicitChecks = argResults['omit-implicit-checks'];
bool trustTypeAnnotations = argResults['trust-type-annotations'];

String file = argResults.rest.first;
Uri entryPoint = Uri.base.resolve(nativeToUriPath(file));
Expand All @@ -48,9 +45,6 @@ show(ArgResults argResults, DataComputer dataComputer,
}

options = new List<String>.from(options);
if (trustTypeAnnotations) {
options.add(Flags.trustTypeAnnotations);
}
if (omitImplicitChecks) {
options.add(Flags.omitImplicitChecks);
}
Expand Down
4 changes: 0 additions & 4 deletions tests/compiler/dart2js/helpers/compiler_helper.dart
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,6 @@ Future<String> compile(String code,

Future<String> compileAll(String code,
{bool disableInlining: true,
bool trustTypeAnnotations: false,
bool minify: false,
int expectedErrors,
int expectedWarnings}) async {
Expand All @@ -99,9 +98,6 @@ Future<String> compileAll(String code,
if (disableInlining) {
options.add(Flags.disableInlining);
}
if (trustTypeAnnotations) {
options.add(Flags.trustTypeAnnotations);
}
if (minify) {
options.add(Flags.minify);
}
Expand Down
Loading

0 comments on commit 6d4d3c6

Please sign in to comment.