azalea/kotlin-fork
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Review fixes for mutable variable smart casting (mostly refactoring).

Browse Source 
DataFlowValue.isPredictable() introduced instead of isStableIdentifier().
Parenthesized assignments are treated correctly.
This commit is contained in:
Mikhail Glukhikh
2015-04-16 20:05:46 +03:00
parent e97e792674
commit 109e0abb6d
17 changed files with 133 additions and 81 deletions
Download Patch File Download Diff File Expand all files Collapse all files
compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/CandidateResolver.java
+1 -1
View File
@@ -446,7 +446,7 @@ public class CandidateResolver {
if (deparenthesizedArgument == null || type == null) return type;
DataFlowValue dataFlowValue = DataFlowValueFactory.createDataFlowValue(deparenthesizedArgument, type, context);
if (!dataFlowValue.isStableIdentifier() && !dataFlowValue.isLocalVariable()) return type;
if (!dataFlowValue.isPredictable()) return type;
Set<JetType> possibleTypes = context.dataFlowInfo.getPossibleTypes(dataFlowValue);
if (possibleTypes.isEmpty()) return type;
compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/smartcasts/DataFlowValue.java
+20 -6
View File
@@ -33,15 +33,17 @@ public class DataFlowValue {
public static final DataFlowValue ERROR = new DataFlowValue(new Object(), ErrorUtils.createErrorType("Error type for data flow"), false, false, Nullability.IMPOSSIBLE);
private final boolean stableIdentifier;
private final boolean localVariable;
private final boolean uncapturedlocalVariable;
private final JetType type;
private final Object id;
private final Nullability immanentNullability;
// Use DataFlowValueFactory
/*package*/ DataFlowValue(Object id, JetType type, boolean stableIdentifier, boolean localVariable, Nullability immanentNullability) {
/*package*/ DataFlowValue(Object id, JetType type, boolean stableIdentifier, boolean uncapturedlocalVariable, Nullability immanentNullability) {
assert !stableIdentifier || !uncapturedlocalVariable :
"data flow value cannot be together a stable identifier and an uncaptured local variable";
this.stableIdentifier = stableIdentifier;
this.localVariable = localVariable;
this.uncapturedlocalVariable = uncapturedlocalVariable;
this.type = type;
this.id = id;
this.immanentNullability = immanentNullability;
@@ -59,16 +61,28 @@ public class DataFlowValue {
/**
* Stable identifier is a non-literal value that is statically known to be immutable
*
* NB: this function is no longer public!
* If you are checking for a possible smart cast, probably you need isPredictable() instead
*/
public boolean isStableIdentifier() {
private boolean isStableIdentifier() {
return stableIdentifier;
}
/**
* Identifier is considered a local variable here if it's mutable (var), local and not captured in a closure
*/
public boolean isLocalVariable() {
return localVariable;
public boolean isUncapturedLocalVariable() {
return uncapturedlocalVariable;
}
/**
* Both stable identifiers and uncaptured local variables are considered "predictable".
* Predictable means here we do not expect some sudden change of their values,
* like accessing mutable properties in another thread or mutable variables from closures.
*/
public boolean isPredictable() {
return stableIdentifier || uncapturedlocalVariable;
}
@NotNull
compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/smartcasts/DataFlowValueFactory.java
+14 -11
View File
@@ -130,7 +130,7 @@ public class DataFlowValueFactory {
JetType type = variableDescriptor.getType();
return new DataFlowValue(variableDescriptor, type,
isStableVariable(variableDescriptor, usageContainingModule),
isLocalVariable(variableDescriptor, bindingContext),
isUncapturedLocalVariable(variableDescriptor, bindingContext),
getImmanentNullability(type));
}
@@ -165,6 +165,11 @@ public class DataFlowValueFactory {
return new IdentifierInfo(id, isStable, isLocal, false);
}
@NotNull
private static IdentifierInfo createStableInfo(Object id) {
return createInfo(id, true, false);
}
@NotNull
private static IdentifierInfo createPackageInfo(Object id) {
return new IdentifierInfo(id, true, false, true);
@@ -180,6 +185,7 @@ public class DataFlowValueFactory {
}
return createInfo(Pair.create(receiverInfo.id, selectorInfo.id),
receiverInfo.isStable && selectorInfo.isStable,
// x.y can never be a local variable
false);
}
@@ -240,7 +246,7 @@ public class DataFlowValueFactory {
VariableDescriptor variableDescriptor = (VariableDescriptor) declarationDescriptor;
return combineInfo(receiverInfo, createInfo(variableDescriptor,
isStableVariable(variableDescriptor, usageModuleDescriptor),
isLocalVariable(variableDescriptor, bindingContext)));
isUncapturedLocalVariable(variableDescriptor, bindingContext)));
}
if (declarationDescriptor instanceof PackageViewDescriptor) {
return createPackageInfo(declarationDescriptor);
@@ -267,21 +273,18 @@ public class DataFlowValueFactory {
ReceiverParameterDescriptor receiverParameter = ((CallableDescriptor) descriptorOfThisReceiver).getExtensionReceiverParameter();
assert receiverParameter != null : "'This' refers to the callable member without a receiver parameter: " +
descriptorOfThisReceiver;
return createInfo(receiverParameter.getValue(), true, false);
return createStableInfo(receiverParameter.getValue());
}
if (descriptorOfThisReceiver instanceof ClassDescriptor) {
return createInfo(((ClassDescriptor) descriptorOfThisReceiver).getThisAsReceiverParameter().getValue(), true, false);
return createStableInfo(((ClassDescriptor) descriptorOfThisReceiver).getThisAsReceiverParameter().getValue());
}
return NO_IDENTIFIER_INFO;
}
public static boolean isLocalVariable(@NotNull VariableDescriptor variableDescriptor, @NotNull BindingContext bindingContext) {
if (variableDescriptor.isVar() && variableDescriptor instanceof LocalVariableDescriptor) {
if (BindingContextUtils.isVarCapturedInClosure(bindingContext, variableDescriptor))
return false;
return true;
}
return false;
public static boolean isUncapturedLocalVariable(@NotNull VariableDescriptor variableDescriptor, @NotNull BindingContext bindingContext) {
return variableDescriptor.isVar()
&& variableDescriptor instanceof LocalVariableDescriptor
&& !BindingContextUtils.isVarCapturedInClosure(bindingContext, variableDescriptor);
}
/**
compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/smartcasts/DelegatingDataFlowInfo.java
+17 -25
View File
@@ -39,14 +39,12 @@ import static org.jetbrains.kotlin.resolve.calls.smartcasts.Nullability.NOT_NULL
@NotNull
private final ImmutableMap<DataFlowValue, Nullability> nullabilityInfo;
/**
* Also immutable
*/
// Also immutable
@NotNull
private final SetMultimap<DataFlowValue, JetType> typeInfo;
/**
* Value for which type info was cleared at this point
* Value for which type info was cleared or reassigned at this point
* so parent type info should not be in use
*/
@Nullable
@@ -94,16 +92,16 @@ import static org.jetbrains.kotlin.resolve.calls.smartcasts.Nullability.NOT_NULL
@NotNull
public SetMultimap<DataFlowValue, JetType> getCompleteTypeInfo() {
SetMultimap<DataFlowValue, JetType> result = newTypeInfo();
Set<DataFlowValue> resultCompleted = new HashSet<DataFlowValue>();
Set<DataFlowValue> withGivenTypeInfo = new HashSet<DataFlowValue>();
DelegatingDataFlowInfo info = this;
while (info != null) {
for (DataFlowValue key : info.typeInfo.keySet()) {
if (!resultCompleted.contains(key)) {
if (!withGivenTypeInfo.contains(key)) {
result.putAll(key, info.typeInfo.get(key));
}
}
if (valueWithGivenTypeInfo != null) {
resultCompleted.add(valueWithGivenTypeInfo);
withGivenTypeInfo.add(valueWithGivenTypeInfo);
}
info = (DelegatingDataFlowInfo) info.parent;
}
@@ -113,7 +111,7 @@ import static org.jetbrains.kotlin.resolve.calls.smartcasts.Nullability.NOT_NULL
@Override
@NotNull
public Nullability getNullability(@NotNull DataFlowValue key) {
if (!key.isStableIdentifier() && !key.isLocalVariable()) return key.getImmanentNullability();
if (!key.isPredictable()) return key.getImmanentNullability();
Nullability nullability = nullabilityInfo.get(key);
return nullability != null ? nullability :
parent != null ? parent.getNullability(key) :
@@ -125,7 +123,7 @@ import static org.jetbrains.kotlin.resolve.calls.smartcasts.Nullability.NOT_NULL
@NotNull DataFlowValue value,
@NotNull Nullability nullability
) {
if (!value.isStableIdentifier() && !value.isLocalVariable()) return false;
if (!value.isPredictable()) return false;
map.put(value, nullability);
return nullability != getNullability(value);
}
@@ -160,14 +158,8 @@ import static org.jetbrains.kotlin.resolve.calls.smartcasts.Nullability.NOT_NULL
@NotNull
public DataFlowInfo clearValueInfo(@NotNull DataFlowValue value) {
Map<DataFlowValue, Nullability> builder = Maps.newHashMap();
boolean changed = putNullability(builder, value, Nullability.UNKNOWN);
// We want to clear all these types
if (!changed) {
changed = !collectTypesFromMeAndParents(value).isEmpty();
}
return !changed
? this
: new DelegatingDataFlowInfo(
putNullability(builder, value, Nullability.UNKNOWN);
return new DelegatingDataFlowInfo(
this,
ImmutableMap.copyOf(builder),
EMPTY_TYPE_INFO,
@@ -178,23 +170,23 @@ import static org.jetbrains.kotlin.resolve.calls.smartcasts.Nullability.NOT_NULL
@Override
@NotNull
public DataFlowInfo assign(@NotNull DataFlowValue a, @NotNull DataFlowValue b) {
Map<DataFlowValue, Nullability> builder = Maps.newHashMap();
Map<DataFlowValue, Nullability> nullability = Maps.newHashMap();
Nullability nullabilityOfB = getNullability(b);
boolean changed = putNullability(builder, a, nullabilityOfB);
putNullability(nullability, a, nullabilityOfB);
SetMultimap<DataFlowValue, JetType> newTypeInfo = newTypeInfo();
Set<JetType> typesForA = collectTypesFromMeAndParents(a);
Set<JetType> typesForB = collectTypesFromMeAndParents(b);
// Own type of B must be recorded separately, e.g. for a constant
// But if its type is the same as A or it's null, there is no reason to do it
// because usually null type or own type are not saved in this set
if (nullabilityOfB.canBeNonNull() && !a.getType().equals(b.getType())) {
typesForB.add(b.getType());
}
newTypeInfo.putAll(a, typesForB);
changed |= !typesForA.equals(typesForB);
return !changed
? this
: new DelegatingDataFlowInfo(
return new DelegatingDataFlowInfo(
this,
ImmutableMap.copyOf(builder),
ImmutableMap.copyOf(nullability),
newTypeInfo.isEmpty() ? EMPTY_TYPE_INFO : newTypeInfo,
a
);
compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/smartcasts/SmartCastUtils.java
+1 -5
View File
@@ -179,11 +179,7 @@ public class SmartCastUtils {
JetExpression expression = ((ExpressionReceiver) receiver).getExpression();
DataFlowValue dataFlowValue = DataFlowValueFactory.createDataFlowValue(receiver, context);
recordCastOrError(expression,
smartCastSubType,
context.trace,
dataFlowValue.isStableIdentifier() || dataFlowValue.isLocalVariable(),
true);
recordCastOrError(expression, smartCastSubType, context.trace, dataFlowValue.isPredictable(), true);
return true;
}
compiler/frontend/src/org/jetbrains/kotlin/types/expressions/ControlStructureTypingVisitor.java
+9 -8
View File
@@ -201,9 +201,10 @@ public class ControlStructureTypingVisitor extends ExpressionTypingVisitor {
ExpressionTypingContext context = contextWithExpectedType.replaceExpectedType(NO_EXPECTED_TYPE).replaceContextDependency(
INDEPENDENT);
// Preliminary analysis
PreliminaryLoopVisitor loopVisitor = new PreliminaryLoopVisitor(expression);
loopVisitor.launch();
context = context.replaceDataFlowInfo(loopVisitor.clearDataFlowInfoForAssignedLocalVariables(context.dataFlowInfo));
PreliminaryLoopVisitor loopVisitor = PreliminaryLoopVisitor.visitLoop(expression);
context = context.replaceDataFlowInfo(
loopVisitor.clearDataFlowInfoForAssignedLocalVariables(context.dataFlowInfo)
);
JetExpression condition = expression.getCondition();
// Extract data flow info from condition itself without taking value into account
@@ -288,9 +289,10 @@ public class ControlStructureTypingVisitor extends ExpressionTypingVisitor {
JetExpression body = expression.getBody();
JetScope conditionScope = context.scope;
// Preliminary analysis
PreliminaryLoopVisitor loopVisitor = new PreliminaryLoopVisitor(expression);
loopVisitor.launch();
context = context.replaceDataFlowInfo(loopVisitor.clearDataFlowInfoForAssignedLocalVariables(context.dataFlowInfo));
PreliminaryLoopVisitor loopVisitor = PreliminaryLoopVisitor.visitLoop(expression);
context = context.replaceDataFlowInfo(
loopVisitor.clearDataFlowInfoForAssignedLocalVariables(context.dataFlowInfo)
);
// Here we must record data flow information at the end of the body (or at the first jump, to be precise) and
// .and it with entrance data flow information, because do-while body is executed at least once
// See KT-6283
@@ -354,8 +356,7 @@ public class ControlStructureTypingVisitor extends ExpressionTypingVisitor {
ExpressionTypingContext context =
contextWithExpectedType.replaceExpectedType(NO_EXPECTED_TYPE).replaceContextDependency(INDEPENDENT);
// Preliminary analysis
PreliminaryLoopVisitor loopVisitor = new PreliminaryLoopVisitor(expression);
loopVisitor.launch();
PreliminaryLoopVisitor loopVisitor = PreliminaryLoopVisitor.visitLoop(expression);
context = context.replaceDataFlowInfo(loopVisitor.clearDataFlowInfoForAssignedLocalVariables(context.dataFlowInfo));
JetExpression loopRange = expression.getLoopRange();
compiler/frontend/src/org/jetbrains/kotlin/types/expressions/DataFlowUtils.java
+2 -6
View File
@@ -199,11 +199,7 @@ public class DataFlowUtils {
for (JetType possibleType : c.dataFlowInfo.getPossibleTypes(dataFlowValue)) {
if (JetTypeChecker.DEFAULT.isSubtypeOf(possibleType, c.expectedType)) {
SmartCastUtils.recordCastOrError(expression,
possibleType,
c.trace,
dataFlowValue.isStableIdentifier() || dataFlowValue.isLocalVariable(),
false);
SmartCastUtils.recordCastOrError(expression, possibleType, c.trace, dataFlowValue.isPredictable(), false);
return possibleType;
}
}
@@ -265,7 +261,7 @@ public class DataFlowUtils {
) {
DataFlowValue dataFlowValue = DataFlowValueFactory.createDataFlowValue(expression, type, c);
Collection<JetType> possibleTypes = Sets.newHashSet(type);
if (dataFlowValue.isStableIdentifier()) {
if (dataFlowValue.isPredictable()) {
possibleTypes.addAll(dataFlowInfo.getPossibleTypes(dataFlowValue));
}
return possibleTypes;
compiler/frontend/src/org/jetbrains/kotlin/types/expressions/ExpressionTypingVisitorForStatements.java
+3
View File
@@ -149,6 +149,8 @@ public class ExpressionTypingVisitorForStatements extends ExpressionTypingVisito
propertyDescriptor, context.trace.getBindingContext(),
DescriptorUtils.getContainingModuleOrNull(scope.getContainingDeclaration()));
DataFlowValue initializerDataFlowValue = DataFlowValueFactory.createDataFlowValue(initializer, type, context);
// We cannot say here anything new about initializerDataFlowValue
// except it has the same value as variableDataFlowValue
dataFlowInfo = dataFlowInfo.assign(variableDataFlowValue, initializerDataFlowValue);
}
}
@@ -350,6 +352,7 @@ public class ExpressionTypingVisitorForStatements extends ExpressionTypingVisito
if (left != null && leftType != null && rightType != null) {
DataFlowValue leftValue = DataFlowValueFactory.createDataFlowValue(left, leftType, context);
DataFlowValue rightValue = DataFlowValueFactory.createDataFlowValue(right, rightType, context);
// We cannot say here anything new about rightValue except it has the same value as leftValue
dataFlowInfo = dataFlowInfo.assign(leftValue, rightValue);
}
}
compiler/frontend/src/org/jetbrains/kotlin/types/expressions/PreliminaryLoopVisitor.java
+17 -16
View File
@@ -36,24 +36,26 @@ class PreliminaryLoopVisitor extends JetTreeVisitor<Void> {
// loop under analysis
private final JetLoopExpression loopExpression;
private Set<Name> assignedNames = new LinkedHashSet<Name>();
private final Set<Name> assignedNames = new LinkedHashSet<Name>();
public PreliminaryLoopVisitor(JetLoopExpression loopExpression) {
private PreliminaryLoopVisitor(@NotNull JetLoopExpression loopExpression) {
this.loopExpression = loopExpression;
}
public void launch() {
loopExpression.accept(this, null);
@NotNull
static public PreliminaryLoopVisitor visitLoop(@NotNull JetLoopExpression loopExpression) {
PreliminaryLoopVisitor visitor = new PreliminaryLoopVisitor(loopExpression);
loopExpression.accept(visitor, null);
return visitor;
}
public DataFlowInfo clearDataFlowInfoForAssignedLocalVariables(DataFlowInfo dataFlowInfo) {
@NotNull
public DataFlowInfo clearDataFlowInfoForAssignedLocalVariables(@NotNull DataFlowInfo dataFlowInfo) {
Map<DataFlowValue, Nullability> nullabilityMap = dataFlowInfo.getCompleteNullabilityInfo();
Set<DataFlowValue> valueSetToClear = new LinkedHashSet<DataFlowValue>();
for (DataFlowValue value: nullabilityMap.keySet()) {
// Only local variables which are not stable are under interest
if (value.isStableIdentifier() || !value.isLocalVariable())
continue;
if (value.getId() instanceof LocalVariableDescriptor) {
// Only uncaptured local variables are under interest here
if (value.isUncapturedLocalVariable() && value.getId() instanceof LocalVariableDescriptor) {
LocalVariableDescriptor descriptor = (LocalVariableDescriptor)value.getId();
if (assignedNames.contains(descriptor.getName())) {
valueSetToClear.add(value);
@@ -66,15 +68,14 @@ class PreliminaryLoopVisitor extends JetTreeVisitor<Void> {
return dataFlowInfo;
}
@Override
public Void visitLoopExpression(@NotNull JetLoopExpression loopExpression, Void arg) {
return super.visitLoopExpression(loopExpression, arg);
}
@Override
public Void visitBinaryExpression(@NotNull JetBinaryExpression binaryExpression, Void arg) {
if (binaryExpression.getOperationToken() == JetTokens.EQ && binaryExpression.getLeft() instanceof JetNameReferenceExpression) {
assignedNames.add(((JetNameReferenceExpression) binaryExpression.getLeft()).getReferencedNameAsName());
if (binaryExpression.getOperationToken() == JetTokens.EQ) {
JetExpression left = JetPsiUtil.deparenthesize(binaryExpression.getLeft());
if (left instanceof JetNameReferenceExpression) {
// deparenthesize
assignedNames.add(((JetNameReferenceExpression) left).getReferencedNameAsName());
}
}
return null;
}
compiler/testData/diagnostics/tests/Casts.kt
+1 -1
View File
@@ -8,7 +8,7 @@ fun test() : Unit {
y <!USELESS_CAST!>as Int<!> : Int
x <!USELESS_CAST!>as Int?<!> : Int?
y <!USELESS_CAST_STATIC_ASSERT_IS_FINE!>as Int?<!> : Int?
x as? Int : Int?
x <!USELESS_CAST_STATIC_ASSERT_IS_FINE!>as? Int<!> : Int?
y <!USELESS_CAST!>as? Int<!> : Int?
x <!USELESS_CAST!>as? Int?<!> : Int?
y <!USELESS_CAST_STATIC_ASSERT_IS_FINE!>as? Int?<!> : Int?
compiler/testData/diagnostics/tests/UnusedVariables.kt
+1 -1
View File
@@ -68,7 +68,7 @@ class MyTest() {
}
else {
a = "ss"
doSmth(a as String)
doSmth(<!DEBUG_INFO_SMARTCAST!>a<!>)
}
doSmth(a)
compiler/testData/diagnostics/tests/smartCasts/varnotnull/whileTrueWithBracketSet.kt
+13
View File
@@ -0,0 +1,13 @@
fun x(): Boolean { return true }
public fun foo(pp: String?): Int {
var p = pp
while(true) {
p!!.length()
if (x()) break
(((p))) = null
}
// Smart cast is NOT possible here
// (we could provide it but p = null makes it much harder)
return p<!UNSAFE_CALL!>.<!>length()
}
compiler/testData/diagnostics/tests/smartCasts/varnotnull/whileTrueWithBracketSet.txt
+4
View File
@@ -0,0 +1,4 @@
package
public fun foo(/*0*/ pp: kotlin.String?): kotlin.Int
internal fun x(): kotlin.Boolean
compiler/testData/diagnostics/tests/smartCasts/varnotnull/whileTrueWithBrackets.kt
+13
View File
@@ -0,0 +1,13 @@
fun x(): Boolean { return true }
public fun foo(pp: String?): Int {
var p = pp
while(true) {
p!!.length()
if (x()) break
(p) = null
}
// Smart cast is NOT possible here
// (we could provide it but p = null makes it much harder)
return p<!UNSAFE_CALL!>.<!>length()
}
compiler/testData/diagnostics/tests/smartCasts/varnotnull/whileTrueWithBrackets.txt
+4
View File
@@ -0,0 +1,4 @@
package
public fun foo(/*0*/ pp: kotlin.String?): kotlin.Int
internal fun x(): kotlin.Boolean
compiler/tests/org/jetbrains/kotlin/checkers/JetDiagnosticsTestGenerated.java
+12
View File
@@ -11786,6 +11786,18 @@ public class JetDiagnosticsTestGenerated extends AbstractJetDiagnosticsTest {
doTest(fileName);
}
@TestMetadata("whileTrueWithBracketSet.kt")
public void testWhileTrueWithBracketSet() throws Exception {
String fileName = JetTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/smartCasts/varnotnull/whileTrueWithBracketSet.kt");
doTest(fileName);
}
@TestMetadata("whileTrueWithBrackets.kt")
public void testWhileTrueWithBrackets() throws Exception {
String fileName = JetTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/smartCasts/varnotnull/whileTrueWithBrackets.kt");
doTest(fileName);
}
@TestMetadata("whileWithBreak.kt")
public void testWhileWithBreak() throws Exception {
String fileName = JetTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/smartCasts/varnotnull/whileWithBreak.kt");
idea/idea-completion/src/org/jetbrains/kotlin/idea/completion/DataFlowInfoUtilForCompletion.kt
+1 -1
View File
@@ -24,7 +24,7 @@ import org.jetbrains.kotlin.descriptors.PackageViewDescriptor
fun renderDataFlowValue(value: DataFlowValue): String? {
// If it is not a stable identifier, there's no point in rendering it
if (!value.isStableIdentifier() && !value.isLocalVariable()) return null
if (!value.isPredictable()) return null
fun renderId(id: Any?): String? {
return when (id) {