Skip to content

Commit cd531d8

Browse files
committed
Control whether a variable is changed in a loop (at this moment there is only a problem with variables captured in a closure)
1 parent 28d07ec commit cd531d8

File tree

4 files changed

+181
-17
lines changed

4 files changed

+181
-17
lines changed

compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/smartcasts/DataFlowInfo.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,13 @@ public interface DataFlowInfo {
4949
@NotNull
5050
Set<JetType> getPossibleTypes(@NotNull DataFlowValue key);
5151

52+
/**
53+
* Call this function to clear all data flow information about
54+
* the given data flow value.
55+
*/
56+
@NotNull
57+
DataFlowInfo clearValueInfo(@NotNull DataFlowValue value);
58+
5259
/**
5360
* Call this function when b is assigned to a
5461
*/

compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/smartcasts/DelegatingDataFlowInfo.java

Lines changed: 74 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@
2929
import static org.jetbrains.kotlin.resolve.calls.smartcasts.Nullability.NOT_NULL;
3030

3131
/* package */ class DelegatingDataFlowInfo implements DataFlowInfo {
32-
private static final ImmutableMap<DataFlowValue,Nullability> EMPTY_NULLABILITY_INFO = ImmutableMap.of();
32+
private static final ImmutableMap<DataFlowValue, Nullability> EMPTY_NULLABILITY_INFO = ImmutableMap.of();
3333
private static final SetMultimap<DataFlowValue, JetType> EMPTY_TYPE_INFO = newTypeInfo();
3434
private static final DataFlowInfo EMPTY_INFO_WITH_JUMP = new DelegatingDataFlowInfo(null, EMPTY_NULLABILITY_INFO,
3535
EMPTY_TYPE_INFO, true);
@@ -40,22 +40,42 @@
4040
@NotNull
4141
private final ImmutableMap<DataFlowValue, Nullability> nullabilityInfo;
4242

43-
/** Also immutable */
43+
/**
44+
* Also immutable
45+
*/
4446
@NotNull
4547
private final SetMultimap<DataFlowValue, JetType> typeInfo;
4648

49+
/**
50+
* Value for which type info was cleared at this point
51+
* so parent type info should not be in use
52+
*/
53+
@Nullable
54+
private final DataFlowValue valueWithoutTypeInfo;
55+
4756
private final boolean jumpPossible;
4857

4958
/* package */ DelegatingDataFlowInfo(
5059
@Nullable DataFlowInfo parent,
5160
@NotNull ImmutableMap<DataFlowValue, Nullability> nullabilityInfo,
5261
@NotNull SetMultimap<DataFlowValue, JetType> typeInfo,
5362
boolean jumpPossible
63+
) {
64+
this(parent, nullabilityInfo, typeInfo, jumpPossible, null);
65+
}
66+
67+
/* package */ DelegatingDataFlowInfo(
68+
@Nullable DataFlowInfo parent,
69+
@NotNull ImmutableMap<DataFlowValue, Nullability> nullabilityInfo,
70+
@NotNull SetMultimap<DataFlowValue, JetType> typeInfo,
71+
boolean jumpPossible,
72+
@Nullable DataFlowValue valueWithoutTypeInfo
5473
) {
5574
this.parent = parent;
5675
this.nullabilityInfo = nullabilityInfo;
5776
this.typeInfo = typeInfo;
5877
this.jumpPossible = jumpPossible;
78+
this.valueWithoutTypeInfo = valueWithoutTypeInfo;
5979
}
6080

6181
@Override
@@ -80,10 +100,16 @@ public Map<DataFlowValue, Nullability> getCompleteNullabilityInfo() {
80100
@NotNull
81101
public SetMultimap<DataFlowValue, JetType> getCompleteTypeInfo() {
82102
SetMultimap<DataFlowValue, JetType> result = newTypeInfo();
103+
Set<DataFlowValue> resultCompleted = new LinkedHashSet<DataFlowValue>();
83104
DelegatingDataFlowInfo info = this;
84105
while (info != null) {
85106
for (DataFlowValue key : info.typeInfo.keySet()) {
86-
result.putAll(key, info.typeInfo.get(key));
107+
if (!resultCompleted.contains(key)) {
108+
result.putAll(key, info.typeInfo.get(key));
109+
}
110+
}
111+
if (valueWithoutTypeInfo != null) {
112+
resultCompleted.add(valueWithoutTypeInfo);
87113
}
88114
info = (DelegatingDataFlowInfo) info.parent;
89115
}
@@ -105,19 +131,25 @@ public Nullability getNullability(@NotNull DataFlowValue key) {
105131
key.getImmanentNullability();
106132
}
107133

108-
private boolean putNullability(@NotNull Map<DataFlowValue, Nullability> map, @NotNull DataFlowValue value, @NotNull Nullability nullability) {
134+
private boolean putNullability(
135+
@NotNull Map<DataFlowValue, Nullability> map,
136+
@NotNull DataFlowValue value,
137+
@NotNull Nullability nullability
138+
) {
109139
if (!value.isStableIdentifier() && !value.isLocalVariable()) return false;
110140
map.put(value, nullability);
111141
return nullability != getNullability(value);
112142
}
113143

114144
@NotNull
115145
private Set<JetType> getDirectlyPossibleTypes(@NotNull DataFlowValue key) {
146+
if (key == valueWithoutTypeInfo) {
147+
return EMPTY_TYPE_INFO.get(key);
148+
}
116149
Set<JetType> theseTypes = typeInfo.get(key);
117-
Set<JetType> types = parent instanceof DelegatingDataFlowInfo ?
118-
Sets.union(theseTypes, ((DelegatingDataFlowInfo)parent).getDirectlyPossibleTypes(key)) :
119-
theseTypes;
120-
return types;
150+
return parent instanceof DelegatingDataFlowInfo ?
151+
Sets.union(theseTypes, ((DelegatingDataFlowInfo) parent).getDirectlyPossibleTypes(key)) :
152+
theseTypes;
121153
}
122154

123155
@Override
@@ -140,6 +172,30 @@ public Set<JetType> getPossibleTypes(@NotNull DataFlowValue key) {
140172
return enrichedTypes;
141173
}
142174

175+
/**
176+
* Call this function to clear all data flow information about
177+
* the given data flow value.
178+
*
179+
* @param value
180+
*/
181+
@NotNull
182+
@Override
183+
public DataFlowInfo clearValueInfo(@NotNull DataFlowValue value) {
184+
Map<DataFlowValue, Nullability> builder = Maps.newHashMap();
185+
boolean changed = putNullability(builder, value, Nullability.UNKNOWN);
186+
// We want to clear all these types
187+
changed |= !getDirectlyPossibleTypes(value).isEmpty();
188+
return !changed
189+
? this
190+
: new DelegatingDataFlowInfo(
191+
this,
192+
ImmutableMap.copyOf(builder),
193+
EMPTY_TYPE_INFO,
194+
jumpPossible,
195+
value
196+
);
197+
}
198+
143199
@NotNull
144200
@Override
145201
public DataFlowInfo assign(@NotNull DataFlowValue a, @NotNull DataFlowValue b) {
@@ -177,13 +233,13 @@ public DataFlowInfo equate(@NotNull DataFlowValue a, @NotNull DataFlowValue b) {
177233
changed |= !newTypeInfo.isEmpty();
178234

179235
return !changed
180-
? this
181-
: new DelegatingDataFlowInfo(
182-
this,
183-
ImmutableMap.copyOf(builder),
184-
newTypeInfo.isEmpty() ? EMPTY_TYPE_INFO : newTypeInfo,
185-
jumpPossible
186-
);
236+
? this
237+
: new DelegatingDataFlowInfo(
238+
this,
239+
ImmutableMap.copyOf(builder),
240+
newTypeInfo.isEmpty() ? EMPTY_TYPE_INFO : newTypeInfo,
241+
jumpPossible
242+
);
187243
}
188244

189245
@NotNull
@@ -254,7 +310,7 @@ public DataFlowInfo and(@NotNull DataFlowInfo otherInfo) {
254310
SetMultimap<DataFlowValue, JetType> myTypeInfo = getCompleteTypeInfo();
255311
SetMultimap<DataFlowValue, JetType> otherTypeInfo = other.getCompleteTypeInfo();
256312
if (nullabilityMapBuilder.isEmpty() && containsAll(myTypeInfo, otherTypeInfo)) {
257-
return otherInfo.isJumpPossible() ? jump(): this;
313+
return otherInfo.isJumpPossible() ? jump() : this;
258314
}
259315

260316
return new DelegatingDataFlowInfo(this, ImmutableMap.copyOf(nullabilityMapBuilder), otherTypeInfo,
@@ -297,7 +353,8 @@ public DataFlowInfo or(@NotNull DataFlowInfo otherInfo) {
297353
return (jumpPossible || otherInfo.isJumpPossible()) ? EMPTY_INFO_WITH_JUMP : EMPTY;
298354
}
299355

300-
return new DelegatingDataFlowInfo(null, ImmutableMap.copyOf(nullabilityMapBuilder), newTypeInfo, jumpPossible || otherInfo.isJumpPossible());
356+
return new DelegatingDataFlowInfo(null, ImmutableMap.copyOf(nullabilityMapBuilder), newTypeInfo,
357+
jumpPossible || otherInfo.isJumpPossible());
301358
}
302359

303360
@NotNull

compiler/frontend/src/org/jetbrains/kotlin/types/expressions/ControlStructureTypingVisitor.java

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,11 @@ public JetTypeInfo visitWhileExpression(JetWhileExpression expression, Expressio
197197

198198
ExpressionTypingContext context = contextWithExpectedType.replaceExpectedType(NO_EXPECTED_TYPE).replaceContextDependency(
199199
INDEPENDENT);
200+
// Preliminary analysis
201+
PreliminaryLoopVisitor loopVisitor = new PreliminaryLoopVisitor(expression, context);
202+
loopVisitor.launch();
203+
context = context.replaceDataFlowInfo(loopVisitor.clearDataFlowInfoForAssignedExpressions());
204+
200205
JetExpression condition = expression.getCondition();
201206
DataFlowInfo dataFlowInfo = checkCondition(context.scope, condition, context);
202207

@@ -275,6 +280,10 @@ public JetTypeInfo visitDoWhileExpression(JetDoWhileExpression expression, Expre
275280
contextWithExpectedType.replaceExpectedType(NO_EXPECTED_TYPE).replaceContextDependency(INDEPENDENT);
276281
JetExpression body = expression.getBody();
277282
JetScope conditionScope = context.scope;
283+
// Preliminary analysis
284+
PreliminaryLoopVisitor loopVisitor = new PreliminaryLoopVisitor(expression, context);
285+
loopVisitor.launch();
286+
context = context.replaceDataFlowInfo(loopVisitor.clearDataFlowInfoForAssignedExpressions());
278287
// Here we must record data flow information at the end of the body (or at the first jump, to be precise) and
279288
// .and it with entrance data flow information, because do-while body is executed at least once
280289
// See KT-6283
@@ -331,6 +340,11 @@ public JetTypeInfo visitForExpression(JetForExpression expression, ExpressionTyp
331340

332341
ExpressionTypingContext context =
333342
contextWithExpectedType.replaceExpectedType(NO_EXPECTED_TYPE).replaceContextDependency(INDEPENDENT);
343+
// Preliminary analysis
344+
PreliminaryLoopVisitor loopVisitor = new PreliminaryLoopVisitor(expression, context);
345+
loopVisitor.launch();
346+
context = context.replaceDataFlowInfo(loopVisitor.clearDataFlowInfoForAssignedExpressions());
347+
334348
JetExpression loopRange = expression.getLoopRange();
335349
JetType expectedParameterType = null;
336350
DataFlowInfo dataFlowInfo = context.dataFlowInfo;
Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
/*
2+
* Copyright 2010-2015 JetBrains s.r.o.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.jetbrains.kotlin.types.expressions;
18+
19+
import org.jetbrains.annotations.NotNull;
20+
import org.jetbrains.kotlin.descriptors.impl.LocalVariableDescriptor;
21+
import org.jetbrains.kotlin.lexer.JetTokens;
22+
import org.jetbrains.kotlin.name.Name;
23+
import org.jetbrains.kotlin.psi.*;
24+
import org.jetbrains.kotlin.resolve.calls.smartcasts.DataFlowInfo;
25+
import org.jetbrains.kotlin.resolve.calls.smartcasts.DataFlowValue;
26+
import org.jetbrains.kotlin.resolve.calls.smartcasts.Nullability;
27+
28+
import java.util.*;
29+
30+
/**
31+
* The purpose of this class is to find all variable assignments
32+
* <b>before</b> loop analysis
33+
*/
34+
class PreliminaryLoopVisitor extends JetTreeVisitor<Void> {
35+
36+
// loop under analysis
37+
private final JetLoopExpression loopExpression;
38+
39+
private final ExpressionTypingContext context;
40+
41+
private Set<Name> assignedNames = new LinkedHashSet<Name>();
42+
43+
public PreliminaryLoopVisitor(JetLoopExpression loopExpression, ExpressionTypingContext context) {
44+
this.loopExpression = loopExpression;
45+
this.context = context;
46+
}
47+
48+
public void launch() {
49+
loopExpression.accept(this, null);
50+
}
51+
52+
public DataFlowInfo clearDataFlowInfoForAssignedExpressions() {
53+
DataFlowInfo dataFlowInfo = context.dataFlowInfo;
54+
Map<DataFlowValue, Nullability> nullabilityMap = dataFlowInfo.getCompleteNullabilityInfo();
55+
Set<DataFlowValue> valueSetToClear = new LinkedHashSet<DataFlowValue>();
56+
for (DataFlowValue value: nullabilityMap.keySet()) {
57+
// Only local variables which are not stable are under interest
58+
if (value.isStableIdentifier() || !value.isLocalVariable())
59+
continue;
60+
if (value.getId() instanceof LocalVariableDescriptor) {
61+
LocalVariableDescriptor descriptor = (LocalVariableDescriptor)value.getId();
62+
if (assignedNames.contains(descriptor.getName())) {
63+
valueSetToClear.add(value);
64+
}
65+
}
66+
}
67+
for (DataFlowValue valueToClear: valueSetToClear) {
68+
dataFlowInfo = dataFlowInfo.clearValueInfo(valueToClear);
69+
}
70+
return dataFlowInfo;
71+
}
72+
73+
@Override
74+
public Void visitLoopExpression(@NotNull JetLoopExpression loopExpression, Void arg) {
75+
return super.visitLoopExpression(loopExpression, arg);
76+
}
77+
78+
@Override
79+
public Void visitBinaryExpression(@NotNull JetBinaryExpression binaryExpression, Void arg) {
80+
if (binaryExpression.getOperationToken() == JetTokens.EQ && binaryExpression.getLeft() instanceof JetNameReferenceExpression) {
81+
assignedNames.add(((JetNameReferenceExpression) binaryExpression.getLeft()).getReferencedNameAsName());
82+
}
83+
return null;
84+
}
85+
86+
}

0 commit comments

Comments
 (0)