From 255e37efb9064129a8a73ff812e6a2deba3e8f17 Mon Sep 17 00:00:00 2001 From: svtk Date: Thu, 9 Feb 2012 12:38:23 +0400 Subject: [PATCH] KT-799 Allow 'return' expressions in conditionals assigned to variables --- .../jet/lang/cfg/JetControlFlowProcessor.java | 13 ---- .../descriptors/VariableDescriptorImpl.java | 4 +- .../jet/lang/resolve/BindingContextUtils.java | 13 +--- .../lang/resolve/DelegatingBindingTrace.java | 9 ++- .../jet/lang/resolve/DescriptorResolver.java | 8 ++- .../jet/lang/resolve/DescriptorUtils.java | 22 +++++++ .../ControlStructureTypingVisitor.java | 60 +++++++++++++------ .../lang/types/expressions/LabelResolver.java | 8 ++- .../jet/util/slicedmap/SlicedMap.java | 7 --- .../jet/util/slicedmap/SlicedMapImpl.java | 5 -- .../jetbrains/jet/util/slicedmap/Slices.java | 9 ++- .../testData/codegen/regressions/kt799.jet | 8 +++ .../diagnostics/tests/FunctionReturnTypes.jet | 4 +- .../testData/diagnostics/tests/Return.jet | 6 +- .../tests/controlStructures/kt799.jet | 25 ++++++++ 15 files changed, 128 insertions(+), 73 deletions(-) create mode 100644 compiler/testData/codegen/regressions/kt799.jet create mode 100644 compiler/testData/diagnostics/tests/controlStructures/kt799.jet diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/cfg/JetControlFlowProcessor.java b/compiler/frontend/src/org/jetbrains/jet/lang/cfg/JetControlFlowProcessor.java index a1566351f28..62c4a24b639 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/cfg/JetControlFlowProcessor.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/cfg/JetControlFlowProcessor.java @@ -578,11 +578,6 @@ public class JetControlFlowProcessor { if (subroutine instanceof JetFunctionLiteralExpression) { subroutine = ((JetFunctionLiteralExpression) subroutine).getFunctionLiteral(); } - boolean error = false; - if (builder.getCurrentSubroutine() != subroutine) { - trace.report(RETURN_NOT_ALLOWED.on(expression)); - error = true; - } if (subroutine instanceof JetFunction || subroutine instanceof JetPropertyAccessor || subroutine instanceof JetSecondaryConstructor) { if (returnedExpression == null) { builder.returnNoValue(expression, subroutine); @@ -591,14 +586,6 @@ public class JetControlFlowProcessor { builder.returnValue(expression, subroutine); } } - else if (!error) { - if (labelElement != null) { - trace.report(NOT_A_RETURN_LABEL.on(expression, labelName)); - } - else { - trace.report(RETURN_NOT_ALLOWED.on(expression)); - } - } } @Override diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/descriptors/VariableDescriptorImpl.java b/compiler/frontend/src/org/jetbrains/jet/lang/descriptors/VariableDescriptorImpl.java index 0766dd99f3f..5cb9dc4310d 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/descriptors/VariableDescriptorImpl.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/descriptors/VariableDescriptorImpl.java @@ -20,7 +20,7 @@ public abstract class VariableDescriptorImpl extends DeclarationDescriptorImpl i @NotNull DeclarationDescriptor containingDeclaration, @NotNull List annotations, @NotNull String name, - @NotNull JetType outType) { + @Nullable JetType outType) { super(containingDeclaration, annotations, name); this.outType = outType; @@ -40,7 +40,7 @@ public abstract class VariableDescriptorImpl extends DeclarationDescriptorImpl i return outType; } - protected void setOutType(JetType outType) { + public void setOutType(JetType outType) { assert this.outType == null; this.outType = outType; } diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/BindingContextUtils.java b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/BindingContextUtils.java index 9635f3edafe..58d89b46f74 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/BindingContextUtils.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/BindingContextUtils.java @@ -3,10 +3,7 @@ package org.jetbrains.jet.lang.resolve; import com.intellij.psi.PsiElement; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; -import org.jetbrains.jet.lang.descriptors.DeclarationDescriptor; -import org.jetbrains.jet.lang.descriptors.FunctionDescriptor; -import org.jetbrains.jet.lang.descriptors.VariableAsFunctionDescriptor; -import org.jetbrains.jet.lang.descriptors.VariableDescriptor; +import org.jetbrains.jet.lang.descriptors.*; import org.jetbrains.jet.lang.psi.*; import org.jetbrains.jet.lang.types.JetType; @@ -49,12 +46,4 @@ public class BindingContextUtils { } return null; } - - @Nullable - public static JetType getFunctionReturnType(@NotNull BindingContext bindingContext, @NotNull JetFunction function) { - DeclarationDescriptor descriptor = bindingContext.get(DECLARATION_TO_DESCRIPTOR, function); - assert descriptor instanceof FunctionDescriptor; - FunctionDescriptor functionDescriptor = (FunctionDescriptor) descriptor; - return functionDescriptor.getReturnType(); - } } diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/DelegatingBindingTrace.java b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/DelegatingBindingTrace.java index 8c18b558dc8..9894f86f16d 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/DelegatingBindingTrace.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/DelegatingBindingTrace.java @@ -57,8 +57,13 @@ public class DelegatingBindingTrace implements BindingTrace { @Override public V get(ReadOnlySlice slice, K key) { - if (map.containsKey(slice, key)) { - return map.get(slice, key); + V value = map.get(slice, key); + if (slice instanceof Slices.SetSlice) { + assert value != null; + if (value.equals(true)) return value; + } + else { + if (value != null) return value; } return parentContext.get(slice, key); } diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/DescriptorResolver.java b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/DescriptorResolver.java index 180672d2388..17d28db95bf 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/DescriptorResolver.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/DescriptorResolver.java @@ -425,13 +425,15 @@ public class DescriptorResolver { @NotNull public VariableDescriptor resolveLocalVariableDescriptor(DeclarationDescriptor containingDeclaration, JetScope scope, JetProperty property, DataFlowInfo dataFlowInfo) { - JetType type = getVariableType(scope, property, dataFlowInfo, false); // For a local variable the type must not be deferred + VariableDescriptorImpl variableDescriptor = resolveLocalVariableDescriptorWithType(containingDeclaration, property, null); - return resolveLocalVariableDescriptorWithType(containingDeclaration, property, type); + JetType type = getVariableType(scope, property, dataFlowInfo, false); // For a local variable the type must not be deferred + variableDescriptor.setOutType(type); + return variableDescriptor; } @NotNull - public VariableDescriptor resolveLocalVariableDescriptorWithType(DeclarationDescriptor containingDeclaration, JetProperty property, JetType type) { + public VariableDescriptorImpl resolveLocalVariableDescriptorWithType(DeclarationDescriptor containingDeclaration, JetProperty property, JetType type) { VariableDescriptorImpl variableDescriptor = new LocalVariableDescriptor( containingDeclaration, annotationResolver.createAnnotationStubs(property.getModifierList()), diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/DescriptorUtils.java b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/DescriptorUtils.java index 51a0b9b2e2f..b5c50e61ecc 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/DescriptorUtils.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/DescriptorUtils.java @@ -5,6 +5,8 @@ import com.google.common.collect.Maps; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.jetbrains.jet.lang.descriptors.*; +import org.jetbrains.jet.lang.psi.*; +import org.jetbrains.jet.lang.resolve.scopes.JetScope; import org.jetbrains.jet.lang.resolve.scopes.receivers.ReceiverDescriptor; import org.jetbrains.jet.lang.types.*; @@ -220,4 +222,24 @@ public class DescriptorUtils { } return descriptor.getClassObjectDescriptor(); } + + @NotNull + public static JetType getFunctionExpectedReturnType(@NotNull FunctionDescriptor descriptor, @NotNull JetElement function) { + JetType expectedType; + if (function instanceof JetFunction) { + if (((JetFunction) function).getReturnTypeRef() != null || ((JetFunction) function).hasBlockBody()) { + expectedType = descriptor.getReturnType(); + } + else { + expectedType = TypeUtils.NO_EXPECTED_TYPE; + } + } + else if (function instanceof JetSecondaryConstructor) { + expectedType = JetStandardClasses.getUnitType(); + } + else { + expectedType = descriptor.getReturnType(); + } + return expectedType != null ? expectedType : TypeUtils.NO_EXPECTED_TYPE; + } } diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/types/expressions/ControlStructureTypingVisitor.java b/compiler/frontend/src/org/jetbrains/jet/lang/types/expressions/ControlStructureTypingVisitor.java index db9e7c054f4..3e7e260c9a2 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/types/expressions/ControlStructureTypingVisitor.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/types/expressions/ControlStructureTypingVisitor.java @@ -4,7 +4,9 @@ import com.intellij.psi.PsiElement; import com.intellij.psi.util.PsiTreeUtil; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import org.jetbrains.jet.lang.descriptors.DeclarationDescriptor; import org.jetbrains.jet.lang.descriptors.FunctionDescriptor; +import org.jetbrains.jet.lang.descriptors.NamedFunctionDescriptor; import org.jetbrains.jet.lang.descriptors.VariableDescriptor; import org.jetbrains.jet.lang.diagnostics.Errors; import org.jetbrains.jet.lang.psi.*; @@ -420,27 +422,51 @@ public class ControlStructureTypingVisitor extends ExpressionTypingVisitor { @Override public JetType visitReturnExpression(JetReturnExpression expression, ExpressionTypingContext context) { - context.labelResolver.recordLabel(expression, context); - if (context.expectedReturnType == TypeUtils.FORBIDDEN) { - context.trace.report(RETURN_NOT_ALLOWED.on(expression)); - return null; - } + JetElement element = context.labelResolver.resolveLabel(expression, context); + JetExpression returnedExpression = expression.getReturnedExpression(); - JetType expectedType = context.expectedReturnType; + JetType expectedType = TypeUtils.NO_EXPECTED_TYPE; + JetExpression parentDeclaration = PsiTreeUtil.getParentOfType(expression, JetDeclaration.class); + if (parentDeclaration instanceof JetFunctionLiteral) { + parentDeclaration = (JetFunctionLiteralExpression) parentDeclaration.getParent(); + } + if (parentDeclaration instanceof JetParameter) { + context.trace.report(RETURN_NOT_ALLOWED.on(expression)); + } + assert parentDeclaration != null; + DeclarationDescriptor declarationDescriptor = context.trace.get(DECLARATION_TO_DESCRIPTOR, parentDeclaration); + FunctionDescriptor containingFunctionDescriptor = DescriptorUtils.getParentOfType(declarationDescriptor, FunctionDescriptor.class, false); + if (expression.getTargetLabel() == null) { - if (PsiTreeUtil.getParentOfType(expression, JetFunctionLiteral.class) == - PsiTreeUtil.getParentOfType(expression, JetDeclaration.class)) { // expression is located in function literal - JetNamedFunction function = JetPsiUtil.getSurroundingFunction(expression); - if (function != null && function.getReturnTypeRef() != null) { - expectedType = BindingContextUtils.getFunctionReturnType(context.trace.getBindingContext(), function); + if (containingFunctionDescriptor != null) { + PsiElement containingFunction = context.trace.get(DESCRIPTOR_TO_DECLARATION, containingFunctionDescriptor); + assert containingFunction != null; + if (containingFunction instanceof JetFunctionLiteralExpression) { + do { + containingFunctionDescriptor = DescriptorUtils.getParentOfType(containingFunctionDescriptor, FunctionDescriptor.class); + containingFunction = containingFunctionDescriptor != null ? context.trace.get(DESCRIPTOR_TO_DECLARATION, containingFunctionDescriptor) : null; + } while (containingFunction instanceof JetFunctionLiteralExpression); + context.trace.report(RETURN_NOT_ALLOWED.on(expression)); + } + if (containingFunctionDescriptor != null) { + expectedType = DescriptorUtils.getFunctionExpectedReturnType(containingFunctionDescriptor, (JetElement) containingFunction); } } + else { + context.trace.report(RETURN_NOT_ALLOWED.on(expression)); + } } - else { - PsiElement element = context.trace.get(LABEL_TARGET, expression.getTargetLabel()); - if (element instanceof JetFunction && ((JetFunction) element).getReturnTypeRef() != null) { - expectedType = BindingContextUtils.getFunctionReturnType(context.trace.getBindingContext(), (JetFunction) element); + else if (element != null) { + NamedFunctionDescriptor functionDescriptor = context.trace.get(FUNCTION, element); + if (functionDescriptor != null) { + expectedType = DescriptorUtils.getFunctionExpectedReturnType(functionDescriptor, element); + if (functionDescriptor != containingFunctionDescriptor) { + context.trace.report(RETURN_NOT_ALLOWED.on(expression)); + } + } + else { + context.trace.report(NOT_A_RETURN_LABEL.on(expression, expression.getLabelName())); } } if (returnedExpression != null) { @@ -456,13 +482,13 @@ public class ControlStructureTypingVisitor extends ExpressionTypingVisitor { @Override public JetType visitBreakExpression(JetBreakExpression expression, ExpressionTypingContext context) { - context.labelResolver.recordLabel(expression, context); + context.labelResolver.resolveLabel(expression, context); return DataFlowUtils.checkType(JetStandardClasses.getNothingType(), expression, context); } @Override public JetType visitContinueExpression(JetContinueExpression expression, ExpressionTypingContext context) { - context.labelResolver.recordLabel(expression, context); + context.labelResolver.resolveLabel(expression, context); return DataFlowUtils.checkType(JetStandardClasses.getNothingType(), expression, context); } } diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/types/expressions/LabelResolver.java b/compiler/frontend/src/org/jetbrains/jet/lang/types/expressions/LabelResolver.java index bcf5b82dcd3..4ac385a045b 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/types/expressions/LabelResolver.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/types/expressions/LabelResolver.java @@ -2,6 +2,7 @@ package org.jetbrains.jet.lang.types.expressions; import com.intellij.psi.PsiElement; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import org.jetbrains.jet.lang.descriptors.ClassDescriptor; import org.jetbrains.jet.lang.descriptors.DeclarationDescriptor; import org.jetbrains.jet.lang.descriptors.FunctionDescriptor; @@ -57,6 +58,7 @@ public class LabelResolver { } } + @Nullable private JetElement resolveControlLabel(@NotNull String labelName, @NotNull JetSimpleNameExpression labelExpression, boolean reportUnresolved, ExpressionTypingContext context) { Collection declarationsByLabel = context.scope.getDeclarationsByLabel(labelName); int size = declarationsByLabel.size(); @@ -80,13 +82,15 @@ public class LabelResolver { return null; } - public void recordLabel(JetLabelQualifiedExpression expression, ExpressionTypingContext context) { + @Nullable + public JetElement resolveLabel(JetLabelQualifiedExpression expression, ExpressionTypingContext context) { JetSimpleNameExpression labelElement = expression.getTargetLabel(); if (labelElement != null) { String labelName = expression.getLabelName(); assert labelName != null; - resolveControlLabel(labelName, labelElement, true, context); + return resolveControlLabel(labelName, labelElement, true, context); } + return null; } private JetElement resolveNamedLabel(@NotNull String labelName, @NotNull JetSimpleNameExpression labelExpression, boolean reportUnresolved, ExpressionTypingContext context) { diff --git a/compiler/frontend/src/org/jetbrains/jet/util/slicedmap/SlicedMap.java b/compiler/frontend/src/org/jetbrains/jet/util/slicedmap/SlicedMap.java index f06598d2cb3..6929fb7eb31 100644 --- a/compiler/frontend/src/org/jetbrains/jet/util/slicedmap/SlicedMap.java +++ b/compiler/frontend/src/org/jetbrains/jet/util/slicedmap/SlicedMap.java @@ -16,11 +16,6 @@ public interface SlicedMap extends Iterable, ?>> { return slice.computeValue(this, key, null, true); } - @Override - public boolean containsKey(ReadOnlySlice slice, K key) { - return false; - } - @Override public Collection getKeys(WritableSlice slice) { return Collections.emptySet(); @@ -34,8 +29,6 @@ public interface SlicedMap extends Iterable, ?>> { V get(ReadOnlySlice slice, K key); - boolean containsKey(ReadOnlySlice slice, K key); - // slice.isCollective() must return true Collection getKeys(WritableSlice slice); } diff --git a/compiler/frontend/src/org/jetbrains/jet/util/slicedmap/SlicedMapImpl.java b/compiler/frontend/src/org/jetbrains/jet/util/slicedmap/SlicedMapImpl.java index 3a67a8e663d..117320b85bf 100644 --- a/compiler/frontend/src/org/jetbrains/jet/util/slicedmap/SlicedMapImpl.java +++ b/compiler/frontend/src/org/jetbrains/jet/util/slicedmap/SlicedMapImpl.java @@ -79,11 +79,6 @@ public class SlicedMapImpl implements MutableSlicedMap { return (Collection) collectiveSliceKeys.get(slice); } - @Override - public boolean containsKey(ReadOnlySlice slice, K key) { - return map.containsKey(slice.makeKey(key)); - } - @Override public V remove(RemovableSlice slice, K key) { //noinspection unchecked diff --git a/compiler/frontend/src/org/jetbrains/jet/util/slicedmap/Slices.java b/compiler/frontend/src/org/jetbrains/jet/util/slicedmap/Slices.java index cd206d72bab..e289b03ed25 100644 --- a/compiler/frontend/src/org/jetbrains/jet/util/slicedmap/Slices.java +++ b/compiler/frontend/src/org/jetbrains/jet/util/slicedmap/Slices.java @@ -105,9 +105,8 @@ public class Slices { public V computeValue(SlicedMap map, K key, V value, boolean valueNotFound) { if (valueNotFound) { for (ReadOnlySlice slice : furtherLookupSlices) { - if (map.containsKey(slice, key)) { - return map.get(slice, key); - } + V v = map.get(slice, key); + if (v != null) return v; } return defaultValue; } @@ -178,8 +177,8 @@ public class Slices { @Override public Boolean computeValue(SlicedMap map, K key, Boolean value, boolean valueNotFound) { - if (valueNotFound) return false; - return super.computeValue(map, key, value, valueNotFound); + Boolean result = super.computeValue(map, key, value, valueNotFound); + return result != null ? result : false; } @Override diff --git a/compiler/testData/codegen/regressions/kt799.jet b/compiler/testData/codegen/regressions/kt799.jet new file mode 100644 index 00000000000..93254b17f6f --- /dev/null +++ b/compiler/testData/codegen/regressions/kt799.jet @@ -0,0 +1,8 @@ +package kt799 + +fun foo(b: Boolean) : String { + val a = if (b) true else return "false" + return "$a" +} + +fun box() = if (foo(true) == "true" && foo(false) == "false") "OK" else "fail" diff --git a/compiler/testData/diagnostics/tests/FunctionReturnTypes.jet b/compiler/testData/diagnostics/tests/FunctionReturnTypes.jet index 79592722ab2..8cbd5644de8 100644 --- a/compiler/testData/diagnostics/tests/FunctionReturnTypes.jet +++ b/compiler/testData/diagnostics/tests/FunctionReturnTypes.jet @@ -7,10 +7,10 @@ fun unitEmpty() : Unit {} fun unitEmptyReturn() : Unit {return} fun unitIntReturn() : Unit {return 1} fun unitUnitReturn() : Unit {return #()} -fun test1() : Any = {return} +fun test1() : Any = {return} fun test2() : Any = @a {return@a 1} fun test3() : Any { return } -fun test4(): ()-> Unit = { return@test4 } +fun test4(): ()-> Unit = { return@test4 } fun test5(): Any = @{ return@ } fun test6(): Any = {return 1} diff --git a/compiler/testData/diagnostics/tests/Return.jet b/compiler/testData/diagnostics/tests/Return.jet index 146783a46fd..a84f385991d 100644 --- a/compiler/testData/diagnostics/tests/Return.jet +++ b/compiler/testData/diagnostics/tests/Return.jet @@ -9,9 +9,9 @@ class A { return@outer } if (1 < 2) - return@A + return@A else if (2 < 3) - return@inner + return@inner return@outer } -} +} \ No newline at end of file diff --git a/compiler/testData/diagnostics/tests/controlStructures/kt799.jet b/compiler/testData/diagnostics/tests/controlStructures/kt799.jet new file mode 100644 index 00000000000..50a04175f3b --- /dev/null +++ b/compiler/testData/diagnostics/tests/controlStructures/kt799.jet @@ -0,0 +1,25 @@ +//KT-799 Allow 'return' expressions in conditionals assigned to variables + +package kt799 + +fun test() { + val a : Int = if (true) 6 else return // should be allowed + + val b = if (true) 6 else return // should be allowed + + doSmth(if (true) 3 else return) + + doSmth(if (true) 3 else return, 1) +} + +val a : Nothing = return 1 + +val b = return 1 + +val c = doSmth(if (true) 3 else return) + + +fun f(mi: Int = if (true) 0 else return) {} + +fun doSmth(i: Int) { +}