Skip to content

Commit

Permalink
Removes cast-check removal logic from compiler.
Browse files Browse the repository at this point in the history
Cast-checking optimization is deprecated with the new replacement
via configurable preconditions:
https://gwt-review.googlesource.com/#/c/10933/

With this patch also the related flag is deprecated and refactored
to set the new corresponding precondition property to keeps existing
apps that sets the flag working.

Change-Id: I15ea663c37d081cb7df58d78c806389571660699
Review-Link: https://gwt-review.googlesource.com/#/c/11330/
  • Loading branch information
gkdn committed Sep 22, 2015
1 parent 3237ac2 commit cd24824
Show file tree
Hide file tree
Showing 17 changed files with 36 additions and 129 deletions.
Expand Up @@ -165,11 +165,6 @@ public File getWorkDir() {
return compileDir.getWorkDir(); return compileDir.getWorkDir();
} }


@Override
public boolean isCastCheckingDisabled() {
return false;
}

@Override @Override
public boolean isClassMetadataDisabled() { public boolean isClassMetadataDisabled() {
return false; return false;
Expand Down
Expand Up @@ -45,11 +45,6 @@ public void setAddRuntimeChecks(boolean enabled) {
throw new UnsupportedOperationException(); throw new UnsupportedOperationException();
} }


@Override
public final void setCastCheckingDisabled(boolean disabled) {
throw new UnsupportedOperationException();
}

@Override @Override
public final void setClassMetadataDisabled(boolean disabled) { public final void setClassMetadataDisabled(boolean disabled) {
throw new UnsupportedOperationException(); throw new UnsupportedOperationException();
Expand Down
11 changes: 1 addition & 10 deletions dev/core/src/com/google/gwt/dev/PrecompileTaskOptionsImpl.java
Expand Up @@ -129,11 +129,6 @@ public String getSourceMapFilePrefix() {
return sourceMapFilePrefix; return sourceMapFilePrefix;
} }


@Override
public boolean isCastCheckingDisabled() {
return jjsOptions.isCastCheckingDisabled();
}

@Override @Override
public boolean isClassMetadataDisabled() { public boolean isClassMetadataDisabled() {
return jjsOptions.isClassMetadataDisabled(); return jjsOptions.isClassMetadataDisabled();
Expand Down Expand Up @@ -213,11 +208,6 @@ public void setAddRuntimeChecks(boolean enabled) {
jjsOptions.setAddRuntimeChecks(enabled); jjsOptions.setAddRuntimeChecks(enabled);
} }


@Override
public void setCastCheckingDisabled(boolean disabled) {
jjsOptions.setCastCheckingDisabled(disabled);
}

@Override @Override
public void setClassMetadataDisabled(boolean disabled) { public void setClassMetadataDisabled(boolean disabled) {
jjsOptions.setClassMetadataDisabled(disabled); jjsOptions.setClassMetadataDisabled(disabled);
Expand Down Expand Up @@ -401,6 +391,7 @@ public boolean shouldJDTInlineCompileTimeConstants() {
public boolean shouldOptimizeDataflow() { public boolean shouldOptimizeDataflow() {
return jjsOptions.shouldOptimizeDataflow(); return jjsOptions.shouldOptimizeDataflow();
} }

@Override @Override
public boolean shouldOrdinalizeEnums() { public boolean shouldOrdinalizeEnums() {
return jjsOptions.shouldOrdinalizeEnums(); return jjsOptions.shouldOrdinalizeEnums();
Expand Down
3 changes: 1 addition & 2 deletions dev/core/src/com/google/gwt/dev/jjs/JJSOptions.java
Expand Up @@ -19,7 +19,6 @@
import com.google.gwt.dev.util.arg.OptionCheckedMode; import com.google.gwt.dev.util.arg.OptionCheckedMode;
import com.google.gwt.dev.util.arg.OptionClosureFormattedOutput; import com.google.gwt.dev.util.arg.OptionClosureFormattedOutput;
import com.google.gwt.dev.util.arg.OptionClusterSimilarFunctions; import com.google.gwt.dev.util.arg.OptionClusterSimilarFunctions;
import com.google.gwt.dev.util.arg.OptionDisableCastChecking;
import com.google.gwt.dev.util.arg.OptionDisableClassMetadata; import com.google.gwt.dev.util.arg.OptionDisableClassMetadata;
import com.google.gwt.dev.util.arg.OptionEnableAssertions; import com.google.gwt.dev.util.arg.OptionEnableAssertions;
import com.google.gwt.dev.util.arg.OptionEnableClosureCompiler; import com.google.gwt.dev.util.arg.OptionEnableClosureCompiler;
Expand Down Expand Up @@ -49,7 +48,7 @@
*/ */
public interface JJSOptions extends OptionOptimize, public interface JJSOptions extends OptionOptimize,
OptionClusterSimilarFunctions, OptionIncrementalCompile, OptionDisableClassMetadata, OptionClusterSimilarFunctions, OptionIncrementalCompile, OptionDisableClassMetadata,
OptionDisableCastChecking, OptionEnableAssertions, OptionInlineLiteralParameters, OptionEnableAssertions, OptionInlineLiteralParameters,
OptionOptimizeDataflow, OptionRunAsyncEnabled, OptionScriptStyle, OptionSoycEnabled, OptionOptimizeDataflow, OptionRunAsyncEnabled, OptionScriptStyle, OptionSoycEnabled,
OptionSoycDetailed, OptionJsonSoycEnabled, OptionOrdinalizeEnums, OptionSoycDetailed, OptionJsonSoycEnabled, OptionOrdinalizeEnums,
OptionRemoveDuplicateFunctions, OptionStrict, OptionRemoveDuplicateFunctions, OptionStrict,
Expand Down
12 changes: 0 additions & 12 deletions dev/core/src/com/google/gwt/dev/jjs/JJSOptionsImpl.java
Expand Up @@ -33,7 +33,6 @@ public class JJSOptionsImpl implements JJSOptions, Serializable {
private boolean clusterSimilarFunctions = true; private boolean clusterSimilarFunctions = true;
private boolean incrementalCompile = false; private boolean incrementalCompile = false;
private boolean compilerMetricsEnabled = false; private boolean compilerMetricsEnabled = false;
private boolean disableCastChecking = false;
private boolean disableClassMetadata = false; private boolean disableClassMetadata = false;
private boolean enableAssertions; private boolean enableAssertions;
private int fragmentCount = -1; private int fragmentCount = -1;
Expand Down Expand Up @@ -63,7 +62,6 @@ public JJSOptionsImpl() {


public void copyFrom(JJSOptions other) { public void copyFrom(JJSOptions other) {
setAddRuntimeChecks(other.shouldAddRuntimeChecks()); setAddRuntimeChecks(other.shouldAddRuntimeChecks());
setCastCheckingDisabled(other.isCastCheckingDisabled());
setClassMetadataDisabled(other.isClassMetadataDisabled()); setClassMetadataDisabled(other.isClassMetadataDisabled());
setClosureCompilerEnabled(other.isClosureCompilerEnabled()); setClosureCompilerEnabled(other.isClosureCompilerEnabled());
setClusterSimilarFunctions(other.shouldClusterSimilarFunctions()); setClusterSimilarFunctions(other.shouldClusterSimilarFunctions());
Expand Down Expand Up @@ -126,11 +124,6 @@ public SourceLevel getSourceLevel() {
return sourceLevel; return sourceLevel;
} }


@Override
public boolean isCastCheckingDisabled() {
return disableCastChecking;
}

@Override @Override
public boolean isClassMetadataDisabled() { public boolean isClassMetadataDisabled() {
return disableClassMetadata; return disableClassMetadata;
Expand Down Expand Up @@ -190,11 +183,6 @@ public void setAddRuntimeChecks(boolean enabled) {
addRuntimeChecks = enabled; addRuntimeChecks = enabled;
} }


@Override
public void setCastCheckingDisabled(boolean disabled) {
disableCastChecking = disabled;
}

@Override @Override
public void setClassMetadataDisabled(boolean disabled) { public void setClassMetadataDisabled(boolean disabled) {
disableClassMetadata = disabled; disableClassMetadata = disabled;
Expand Down
Expand Up @@ -481,13 +481,11 @@ protected TypeMapper<?> normalizeSemantics() {
ComputeExhaustiveCastabilityInformation.exec(jprogram); ComputeExhaustiveCastabilityInformation.exec(jprogram);
} else { } else {
// If trivial casts are pruned then one can use smaller runtime castmaps. // If trivial casts are pruned then one can use smaller runtime castmaps.
ComputeCastabilityInformation.exec(jprogram, options.isCastCheckingDisabled(), ComputeCastabilityInformation.exec(jprogram, !shouldOptimize() /* recordTrivialCasts */);
!shouldOptimize() /* recordTrivialCasts */);
} }


ImplementCastsAndTypeChecks.exec(jprogram, options.isCastCheckingDisabled(), ImplementCastsAndTypeChecks.exec(jprogram, shouldOptimize() /* pruneTrivialCasts */);
shouldOptimize() /* pruneTrivialCasts */); ArrayNormalizer.exec(jprogram);
ArrayNormalizer.exec(jprogram, options.isCastCheckingDisabled());
EqualityNormalizer.exec(jprogram); EqualityNormalizer.exec(jprogram);


TypeMapper<?> typeMapper = getTypeMapper(); TypeMapper<?> typeMapper = getTypeMapper();
Expand Down
12 changes: 4 additions & 8 deletions dev/core/src/com/google/gwt/dev/jjs/impl/ArrayNormalizer.java
Expand Up @@ -49,8 +49,7 @@ private class ArrayVisitor extends JModVisitor {


@Override @Override
public void endVisit(JBinaryOperation x, Context ctx) { public void endVisit(JBinaryOperation x, Context ctx) {
if (disableCastChecking || x.getOp() != JBinaryOperator.ASG || if (x.getOp() != JBinaryOperator.ASG || !(x.getLhs() instanceof JArrayRef)) {
!(x.getLhs() instanceof JArrayRef)) {
return; return;
} }
JArrayRef arrayRef = (JArrayRef) x.getLhs(); JArrayRef arrayRef = (JArrayRef) x.getLhs();
Expand Down Expand Up @@ -202,22 +201,19 @@ private JIntLiteral getTypeCategoryLiteral(JType type) {
} }
} }


public static void exec(JProgram program, boolean disableCastChecking) { public static void exec(JProgram program) {
new ArrayNormalizer(program, disableCastChecking).execImpl(); new ArrayNormalizer(program).execImpl();
} }


private final boolean disableCastChecking;
private final JMethod initDim; private final JMethod initDim;
private final JMethod initDims; private final JMethod initDims;
private final JMethod initValues; private final JMethod initValues;
private final JProgram program; private final JProgram program;
private final JMethod setCheckMethod; private final JMethod setCheckMethod;


private ArrayNormalizer(JProgram program, boolean disableCastChecking) { private ArrayNormalizer(JProgram program) {
this.program = program; this.program = program;
this.disableCastChecking = disableCastChecking;
setCheckMethod = program.getIndexedMethod("Array.setCheck"); setCheckMethod = program.getIndexedMethod("Array.setCheck");

initDim = program.getIndexedMethod("Array.initDim"); initDim = program.getIndexedMethod("Array.initDim");
initDims = program.getIndexedMethod("Array.initDims"); initDims = program.getIndexedMethod("Array.initDims");
initValues = program.getIndexedMethod("Array.initValues"); initValues = program.getIndexedMethod("Array.initValues");
Expand Down
Expand Up @@ -151,7 +151,7 @@ public void endVisit(JBinaryOperation x, Context ctx) {


@Override @Override
public void endVisit(JCastOperation x, Context ctx) { public void endVisit(JCastOperation x, Context ctx) {
if (disableCastChecking || x.getCastType().isNullType()) { if (x.getCastType().isNullType()) {
return; return;
} }
recordCast(x.getCastType(), x.getExpr()); recordCast(x.getCastType(), x.getExpr());
Expand Down Expand Up @@ -288,28 +288,23 @@ private void recordCastInternal(JReferenceType toType, JReferenceType rhsType) {
} }
} }


public static void exec(JProgram program, boolean disableCastChecking, public static void exec(JProgram program, boolean recordTrivialCasts) {
boolean recordTrivialCasts) { new ComputeCastabilityInformation(program, recordTrivialCasts).execImpl();
new ComputeCastabilityInformation(program, disableCastChecking, recordTrivialCasts).execImpl();
} }


public static void exec(JProgram program, boolean disableCastChecking) { public static void exec(JProgram program) {
new ComputeCastabilityInformation(program, disableCastChecking, false).execImpl(); new ComputeCastabilityInformation(program, false).execImpl();
} }


private final boolean disableCastChecking;

private final boolean recordTrivialCasts; private final boolean recordTrivialCasts;


private final JProgram program; private final JProgram program;


private final JTypeOracle typeOracle; private final JTypeOracle typeOracle;


private ComputeCastabilityInformation(JProgram program, boolean disableCastChecking, private ComputeCastabilityInformation(JProgram program, boolean recordTrivialCasts) {
boolean recordTrivialCasts) {
this.program = program; this.program = program;
this.typeOracle = program.typeOracle; this.typeOracle = program.typeOracle;
this.disableCastChecking = disableCastChecking;
this.recordTrivialCasts = recordTrivialCasts; this.recordTrivialCasts = recordTrivialCasts;
} }


Expand Down
Expand Up @@ -57,11 +57,6 @@ public void endVisit(JCastOperation x, Context ctx) {
JType toType = x.getCastType(); JType toType = x.getCastType();
JExpression expr = x.getExpr(); JExpression expr = x.getExpr();


// Even if disableCastChecking is enabled, we need to rescue JSOs
if (disableCastChecking && toType instanceof JReferenceType) {
// Just leave the cast in, GenerateJavaScriptAST will ignore it.
return;
}
SourceInfo info = x.getSourceInfo(); SourceInfo info = x.getSourceInfo();
if (pruneTrivialCasts && toType.isNullType()) { if (pruneTrivialCasts && toType.isNullType()) {
/** /**
Expand Down Expand Up @@ -268,16 +263,14 @@ private JMethodCall implementCastOrInstanceOfOperation(SourceInfo sourceInfo,
return call; return call;
} }


public static void exec(JProgram program, boolean disableCastChecking, public static void exec(JProgram program, boolean pruneTrivialCasts) {
boolean pruneTrivialCasts) { new ImplementCastsAndTypeChecks(program, pruneTrivialCasts).execImpl();
new ImplementCastsAndTypeChecks(program, disableCastChecking, pruneTrivialCasts).execImpl();
} }


public static void exec(JProgram program, boolean disableCastChecking) { public static void exec(JProgram program) {
new ImplementCastsAndTypeChecks(program, disableCastChecking, true).execImpl(); new ImplementCastsAndTypeChecks(program, true).execImpl();
} }


private final boolean disableCastChecking;
private final boolean pruneTrivialCasts; private final boolean pruneTrivialCasts;
private final JProgram program; private final JProgram program;


Expand All @@ -287,10 +280,8 @@ public static void exec(JProgram program, boolean disableCastChecking) {
private Map<TypeCategory, JMethod> dynamicCastMethodsByTargetTypeCategory = private Map<TypeCategory, JMethod> dynamicCastMethodsByTargetTypeCategory =
Maps.newEnumMap(TypeCategory.class); Maps.newEnumMap(TypeCategory.class);


private ImplementCastsAndTypeChecks(JProgram program, boolean disableCastChecking, private ImplementCastsAndTypeChecks(JProgram program, boolean pruneTrivialCasts) {
boolean pruneTrivialCasts) {
this.program = program; this.program = program;
this.disableCastChecking = disableCastChecking;
this.pruneTrivialCasts = pruneTrivialCasts; this.pruneTrivialCasts = pruneTrivialCasts;


// Populate the necessary instanceOf methods. // Populate the necessary instanceOf methods.
Expand Down
Expand Up @@ -17,22 +17,24 @@


import com.google.gwt.util.tools.ArgHandlerFlag; import com.google.gwt.util.tools.ArgHandlerFlag;


import java.util.Arrays;
import java.util.List;

/** /**
* Enables run-time cast checking. * Enables run-time cast checking.
*/ */
public class ArgHandlerDisableCastChecking extends ArgHandlerFlag { public class ArgHandlerDisableCastChecking extends ArgHandlerFlag {


private final OptionDisableCastChecking option; private OptionSetProperties setProperties;

public ArgHandlerDisableCastChecking(OptionDisableCastChecking option) {
this.option = option;


public ArgHandlerDisableCastChecking(OptionSetProperties setProperties) {
this.setProperties = setProperties;
addTagValue("-XdisableCastChecking", false); addTagValue("-XdisableCastChecking", false);
} }


@Override @Override
public String getPurposeSnippet() { public String getPurposeSnippet() {
return "Insert run-time checking of cast operations."; return "DEPRECATED: use checks.checkLevel instead.";
} }


@Override @Override
Expand All @@ -42,7 +44,8 @@ public String getLabel() {


@Override @Override
public boolean setFlag(boolean value) { public boolean setFlag(boolean value) {
option.setCastCheckingDisabled(!value); List<String> propertyValue = Arrays.asList(value ? "ENABLED" : "DISABLED");
setProperties.setPropertyValues("checks.type", propertyValue);
return true; return true;
} }


Expand All @@ -53,6 +56,6 @@ public boolean isExperimental() {


@Override @Override
public boolean getDefaultValue() { public boolean getDefaultValue() {
return !option.isCastCheckingDisabled(); return false;
} }
} }

This file was deleted.

Expand Up @@ -44,7 +44,7 @@ public boolean isUndocumented() {


@Override @Override
public String getPurpose() { public String getPurpose() {
return "Deprecated. Has no effect and will be removed in a future release"; return "DEPRECATED: Has no effect and will be removed in a future release";
} }


@Override @Override
Expand Down
Expand Up @@ -38,7 +38,7 @@ protected void setUp() throws Exception {
public void testFlagBackwardCompatibility() { public void testFlagBackwardCompatibility() {
// Set a bunch of boolean flags using old-style tags. // Set a bunch of boolean flags using old-style tags.
precompileTaskArgProcessor.processArgs("-workDir", "/tmp", "-XcompilerMetrics", precompileTaskArgProcessor.processArgs("-workDir", "/tmp", "-XcompilerMetrics",
"-XdisableCastChecking", "-XdisableClassMetadata", "-XdisableClusterSimilarFunctions", "-XdisableClassMetadata", "-XdisableClusterSimilarFunctions",
"-XdisableInlineLiteralParameters", "-XdisableOptimizeDataflow", "-XdisableOrdinalizeEnums", "-XdisableInlineLiteralParameters", "-XdisableOptimizeDataflow", "-XdisableOrdinalizeEnums",
"-XdisableRemoveDuplicateFunctions", "-XdisableRunAsync", "-XdisableSoycHtml", "-XdisableRemoveDuplicateFunctions", "-XdisableRunAsync", "-XdisableSoycHtml",
"-XdisableUpdateCheck", "-ea", "-XenableClosureCompiler", "-soyc", "-XsoycDetailed", "-XdisableUpdateCheck", "-ea", "-XenableClosureCompiler", "-soyc", "-XsoycDetailed",
Expand All @@ -47,8 +47,6 @@ public void testFlagBackwardCompatibility() {
// Show that the flags were recognized and ended up modifying options. // Show that the flags were recognized and ended up modifying options.
assertNotEquals( assertNotEquals(
defaultOptions.isCompilerMetricsEnabled(), handledOptions.isCompilerMetricsEnabled()); defaultOptions.isCompilerMetricsEnabled(), handledOptions.isCompilerMetricsEnabled());
assertNotEquals(
defaultOptions.isCastCheckingDisabled(), handledOptions.isCastCheckingDisabled());
assertNotEquals( assertNotEquals(
defaultOptions.isClassMetadataDisabled(), handledOptions.isClassMetadataDisabled()); defaultOptions.isClassMetadataDisabled(), handledOptions.isClassMetadataDisabled());
assertNotEquals(defaultOptions.shouldClusterSimilarFunctions(), assertNotEquals(defaultOptions.shouldClusterSimilarFunctions(),
Expand Down
Expand Up @@ -21,7 +21,6 @@
* Test for {@link ArrayNormalizer}. * Test for {@link ArrayNormalizer}.
*/ */
public class ArrayNormalizerTest extends OptimizerTestBase { public class ArrayNormalizerTest extends OptimizerTestBase {
private boolean disableCastCheck = false;
// TODO(rluble): add unit test for the rest of the functionality. // TODO(rluble): add unit test for the rest of the functionality.


public void testSetCheckElimination_finalTypeArray() throws Exception { public void testSetCheckElimination_finalTypeArray() throws Exception {
Expand All @@ -35,21 +34,6 @@ public void testSetCheckElimination_finalTypeArray() throws Exception {
"a[1] = new EntryPoint$A();"); "a[1] = new EntryPoint$A();");
} }


public void testSetCheckElimination_disableCastCheck() throws Exception {
disableCastCheck = true;
addSnippetClassDecl("static class A {String name; public void set() { name = \"A\";} }");
addSnippetClassDecl("static class B extends A { }");

Result result =
optimize("void", "A[] a = new A[1]; a = new B[1]; a[1] = new B();");
result.intoString(
"EntryPoint$A[] a = Array.initDim(EntryPoint$A.class, , " +
"/* JRuntimeTypeReference */\"test.EntryPoint$A\", 1, 0, 1);",
"a = Array.initDim(EntryPoint$B.class, , " +
"/* JRuntimeTypeReference */\"test.EntryPoint$B\", 1, 0, 1);",
"a[1] = new EntryPoint$B();");
}

public void testSetCheckPreservation_nonFinalTypeArray() throws Exception { public void testSetCheckPreservation_nonFinalTypeArray() throws Exception {
addSnippetClassDecl("static class A {String name; public void set() { name = \"A\";} }"); addSnippetClassDecl("static class A {String name; public void set() { name = \"A\";} }");
addSnippetClassDecl("static class B extends A { }"); addSnippetClassDecl("static class B extends A { }");
Expand All @@ -74,7 +58,7 @@ protected boolean doOptimizeMethod(TreeLogger logger, JProgram program, JMethod
didChange &= TypeTightener.exec(program).didChange(); didChange &= TypeTightener.exec(program).didChange();
didChange &= MethodCallTightener.exec(program).didChange(); didChange &= MethodCallTightener.exec(program).didChange();
} while (didChange); } while (didChange);
ArrayNormalizer.exec(program, disableCastCheck); ArrayNormalizer.exec(program);
return true; return true;
} }
} }

0 comments on commit cd24824

Please sign in to comment.