From 70adb0f4e2e4ac25ea2c635a7770163c45934d03 Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Fri, 18 Jul 2014 20:03:57 +0400 Subject: [PATCH] Temporarily prohibit non-local returns for some stdlib functions Make InlineUtil work with FQ names, not descriptors #KT-5496 Fixed #KT-5497 Fixed --- ...hibitNonLocalReturnOutOfTryCatchFinally.kt | 42 ++++++++++++++++ ...JetDiagnosticsTestWithStdLibGenerated.java | 16 +++++- .../jet/lang/types/lang/AnnotationUtil.kt | 49 ------------------- .../jet/lang/types/lang/InlineUtil.java | 48 ++++++++++-------- libraries/stdlib/src/kotlin/JLangJVM.kt | 3 +- .../stdlib/src/kotlin/concurrent/Locks.kt | 9 ++-- libraries/stdlib/src/kotlin/io/Closable.kt | 3 +- libraries/stdlib/src/kotlin/io/JIO.kt | 3 +- 8 files changed, 96 insertions(+), 77 deletions(-) create mode 100644 compiler/testData/diagnostics/testsWithStdLib/nonLocalReturns/prohibitNonLocalReturnOutOfTryCatchFinally.kt delete mode 100644 core/descriptors/src/org/jetbrains/jet/lang/types/lang/AnnotationUtil.kt diff --git a/compiler/testData/diagnostics/testsWithStdLib/nonLocalReturns/prohibitNonLocalReturnOutOfTryCatchFinally.kt b/compiler/testData/diagnostics/testsWithStdLib/nonLocalReturns/prohibitNonLocalReturnOutOfTryCatchFinally.kt new file mode 100644 index 00000000000..852927a7878 --- /dev/null +++ b/compiler/testData/diagnostics/testsWithStdLib/nonLocalReturns/prohibitNonLocalReturnOutOfTryCatchFinally.kt @@ -0,0 +1,42 @@ +import java.io.Closeable +import java.util.concurrent.locks.Lock +import java.util.concurrent.locks.ReentrantReadWriteLock +import kotlin.concurrent.* + +// Non-local returns for these functions are temporarily disabled because they contain try-finally blocks and +// the compiler doesn't correctly include the "finally" section into the inlined result. +// Once the compiler is fixed (KT-5506), non-local returns can be re-allowed for these functions + +fun testSynchronized(): Int { + synchronized("") { + return 1 + } +} + + +fun testUse(f: Closeable): Int { + f.use { + return 2 + } +} + + +fun testWithLock(l: Lock): Int { + l.withLock { + return 3 + } +} + + +fun testRead(l: ReentrantReadWriteLock): Int { + l.read { + return 4 + } +} + + +fun testWrite(l: ReentrantReadWriteLock): Int { + l.write { + return 5 + } +} diff --git a/compiler/tests/org/jetbrains/jet/checkers/JetDiagnosticsTestWithStdLibGenerated.java b/compiler/tests/org/jetbrains/jet/checkers/JetDiagnosticsTestWithStdLibGenerated.java index 7bc1d1161ba..75d6fb5c3fa 100644 --- a/compiler/tests/org/jetbrains/jet/checkers/JetDiagnosticsTestWithStdLibGenerated.java +++ b/compiler/tests/org/jetbrains/jet/checkers/JetDiagnosticsTestWithStdLibGenerated.java @@ -31,7 +31,7 @@ import org.jetbrains.jet.checkers.AbstractJetDiagnosticsTestWithStdLib; /** This class is generated by {@link org.jetbrains.jet.generators.tests.TestsPackage}. DO NOT MODIFY MANUALLY */ @SuppressWarnings("all") @TestMetadata("compiler/testData/diagnostics/testsWithStdLib") -@InnerTestClasses({JetDiagnosticsTestWithStdLibGenerated.Annotations.class, JetDiagnosticsTestWithStdLibGenerated.CallableReference.class, JetDiagnosticsTestWithStdLibGenerated.DuplicateJvmSignature.class, JetDiagnosticsTestWithStdLibGenerated.FunctionLiterals.class, JetDiagnosticsTestWithStdLibGenerated.KotlinSignature.class}) +@InnerTestClasses({JetDiagnosticsTestWithStdLibGenerated.Annotations.class, JetDiagnosticsTestWithStdLibGenerated.CallableReference.class, JetDiagnosticsTestWithStdLibGenerated.DuplicateJvmSignature.class, JetDiagnosticsTestWithStdLibGenerated.FunctionLiterals.class, JetDiagnosticsTestWithStdLibGenerated.KotlinSignature.class, JetDiagnosticsTestWithStdLibGenerated.NonLocalReturns.class}) public class JetDiagnosticsTestWithStdLibGenerated extends AbstractJetDiagnosticsTestWithStdLib { public void testAllFilesPresentInTestsWithStdLib() throws Exception { JetTestUtils.assertAllTestsPresentByMetadata(this.getClass(), "org.jetbrains.jet.generators.tests.TestsPackage", new File("compiler/testData/diagnostics/testsWithStdLib"), Pattern.compile("^(.+)\\.kt$"), true); @@ -495,6 +495,19 @@ public class JetDiagnosticsTestWithStdLibGenerated extends AbstractJetDiagnostic } + @TestMetadata("compiler/testData/diagnostics/testsWithStdLib/nonLocalReturns") + public static class NonLocalReturns extends AbstractJetDiagnosticsTestWithStdLib { + public void testAllFilesPresentInNonLocalReturns() throws Exception { + JetTestUtils.assertAllTestsPresentByMetadata(this.getClass(), "org.jetbrains.jet.generators.tests.TestsPackage", new File("compiler/testData/diagnostics/testsWithStdLib/nonLocalReturns"), Pattern.compile("^(.+)\\.kt$"), true); + } + + @TestMetadata("prohibitNonLocalReturnOutOfTryCatchFinally.kt") + public void testProhibitNonLocalReturnOutOfTryCatchFinally() throws Exception { + doTest("compiler/testData/diagnostics/testsWithStdLib/nonLocalReturns/prohibitNonLocalReturnOutOfTryCatchFinally.kt"); + } + + } + public static Test suite() { TestSuite suite = new TestSuite("JetDiagnosticsTestWithStdLibGenerated"); suite.addTestSuite(JetDiagnosticsTestWithStdLibGenerated.class); @@ -503,6 +516,7 @@ public class JetDiagnosticsTestWithStdLibGenerated extends AbstractJetDiagnostic suite.addTestSuite(DuplicateJvmSignature.class); suite.addTestSuite(FunctionLiterals.class); suite.addTestSuite(KotlinSignature.class); + suite.addTestSuite(NonLocalReturns.class); return suite; } } diff --git a/core/descriptors/src/org/jetbrains/jet/lang/types/lang/AnnotationUtil.kt b/core/descriptors/src/org/jetbrains/jet/lang/types/lang/AnnotationUtil.kt deleted file mode 100644 index 2320e316e28..00000000000 --- a/core/descriptors/src/org/jetbrains/jet/lang/types/lang/AnnotationUtil.kt +++ /dev/null @@ -1,49 +0,0 @@ -/* - * Copyright 2010-2014 JetBrains s.r.o. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.jetbrains.jet.lang.types.lang - -import org.jetbrains.jet.lang.descriptors.DeclarationDescriptor -import org.jetbrains.jet.lang.descriptors.ClassDescriptor -import org.jetbrains.jet.lang.descriptors.annotations.AnnotationDescriptor -import org.jetbrains.jet.lang.resolve.constants.CompileTimeConstant -import org.jetbrains.jet.lang.descriptors.annotations.Annotations - -public object AnnotationUtil { - - public fun getAnnotationSingleArgument(descriptor: DeclarationDescriptor, annotationClass: ClassDescriptor) : CompileTimeConstant? { - val annotation = getAnnotation(descriptor.getAnnotations(), annotationClass) - - if (annotation != null) { - val parameterDescriptor = annotationClass.getConstructors().elementAt(0).getValueParameters().first - if (parameterDescriptor != null) { - return annotation.getValueArgument(parameterDescriptor) - } - } - return null - } - - public fun getAnnotation(annotations: Annotations, annotationClass: ClassDescriptor): AnnotationDescriptor? { - for (annotation in annotations) { - if (annotationClass == annotation.getType().getConstructor().getDeclarationDescriptor()) { - return annotation - } - } - return null - } - - -} \ No newline at end of file diff --git a/core/descriptors/src/org/jetbrains/jet/lang/types/lang/InlineUtil.java b/core/descriptors/src/org/jetbrains/jet/lang/types/lang/InlineUtil.java index 9810dcadf71..23d65e1e32f 100644 --- a/core/descriptors/src/org/jetbrains/jet/lang/types/lang/InlineUtil.java +++ b/core/descriptors/src/org/jetbrains/jet/lang/types/lang/InlineUtil.java @@ -16,12 +16,16 @@ package org.jetbrains.jet.lang.types.lang; +import kotlin.KotlinPackage; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import org.jetbrains.jet.lang.descriptors.CallableDescriptor; import org.jetbrains.jet.lang.descriptors.ClassDescriptor; import org.jetbrains.jet.lang.descriptors.DeclarationDescriptor; import org.jetbrains.jet.lang.descriptors.ValueParameterDescriptor; +import org.jetbrains.jet.lang.descriptors.annotations.Annotated; import org.jetbrains.jet.lang.descriptors.annotations.AnnotationDescriptor; +import org.jetbrains.jet.lang.resolve.DescriptorUtils; import org.jetbrains.jet.lang.resolve.constants.ArrayValue; import org.jetbrains.jet.lang.resolve.constants.CompileTimeConstant; import org.jetbrains.jet.lang.resolve.constants.EnumValue; @@ -31,29 +35,23 @@ import java.util.List; public class InlineUtil { public static boolean hasNoinlineAnnotation(@NotNull CallableDescriptor valueParameterDescriptor) { - KotlinBuiltIns builtIns = KotlinBuiltIns.getInstance(); - return KotlinBuiltIns.containsAnnotation(valueParameterDescriptor, builtIns.getNoinlineClassAnnotation()); + return KotlinBuiltIns.containsAnnotation(valueParameterDescriptor, KotlinBuiltIns.getInstance().getNoinlineClassAnnotation()); } @NotNull public static InlineStrategy getInlineType(@NotNull DeclarationDescriptor descriptor) { - ClassDescriptor annotationClass = KotlinBuiltIns.getInstance().getInlineClassAnnotation(); - AnnotationDescriptor annotation = AnnotationUtil.instance$.getAnnotation(descriptor.getAnnotations(), annotationClass); - if (annotation != null) { - CompileTimeConstant argument = AnnotationUtil.instance$.getAnnotationSingleArgument(descriptor, annotationClass); - if (argument == null) { - //default parameter - return InlineStrategy.AS_FUNCTION; - } - else { - assert argument instanceof EnumValue : "Inline annotation parameter should be inline entry but was: " + argument + "!"; - String name = ((EnumValue) argument).getValue().getName().asString(); - return name.equals(InlineStrategy.IN_PLACE.name()) ? InlineStrategy.IN_PLACE : InlineStrategy.AS_FUNCTION; - } - } - else { + ClassDescriptor inlineAnnotation = KotlinBuiltIns.getInstance().getInlineClassAnnotation(); + AnnotationDescriptor annotation = descriptor.getAnnotations().findAnnotation(DescriptorUtils.getFqNameSafe(inlineAnnotation)); + if (annotation == null) { return InlineStrategy.NOT_INLINE; } + CompileTimeConstant argument = getAnnotationSingleArgument(descriptor, inlineAnnotation); + if (argument == null) { + return InlineStrategy.AS_FUNCTION; + } + assert argument instanceof EnumValue : "Inline annotation parameter should be enum entry but was: " + argument; + String name = ((EnumValue) argument).getValue().getName().asString(); + return name.equals(InlineStrategy.IN_PLACE.name()) ? InlineStrategy.IN_PLACE : InlineStrategy.AS_FUNCTION; } public static boolean hasOnlyLocalContinueAndBreak(@NotNull ValueParameterDescriptor descriptor) { @@ -66,8 +64,7 @@ public class InlineUtil { private static boolean hasInlineOption(@NotNull ValueParameterDescriptor descriptor, @NotNull InlineOption option) { CompileTimeConstant argument = - AnnotationUtil.instance$.getAnnotationSingleArgument(descriptor, KotlinBuiltIns.getInstance() - .getInlineOptionsClassAnnotation()); + getAnnotationSingleArgument(descriptor, KotlinBuiltIns.getInstance().getInlineOptionsClassAnnotation()); if (argument instanceof ArrayValue) { List> values = ((ArrayValue) argument).getValue(); @@ -83,5 +80,16 @@ public class InlineUtil { return false; } -} + @Nullable + private static CompileTimeConstant getAnnotationSingleArgument( + @NotNull Annotated annotated, + @NotNull ClassDescriptor annotationClass + ) { + AnnotationDescriptor annotation = annotated.getAnnotations().findAnnotation(DescriptorUtils.getFqNameSafe(annotationClass)); + if (annotation != null) { + return KotlinPackage.firstOrNull(annotation.getAllValueArguments().values()); + } + return null; + } +} diff --git a/libraries/stdlib/src/kotlin/JLangJVM.kt b/libraries/stdlib/src/kotlin/JLangJVM.kt index 8b0db79545d..0feaa0591aa 100644 --- a/libraries/stdlib/src/kotlin/JLangJVM.kt +++ b/libraries/stdlib/src/kotlin/JLangJVM.kt @@ -4,6 +4,7 @@ import java.lang.annotation.Retention import java.lang.annotation.RetentionPolicy import kotlin.jvm.internal.unsafe.* import kotlin.jvm.internal.Intrinsic +import kotlin.InlineOption.ONLY_LOCAL_RETURN /** * This annotation indicates what exceptions should be declared by a function when compiled to a JVM method @@ -25,7 +26,7 @@ public annotation class throws(vararg val exceptionClasses: Class [Intrinsic("kotlin.javaClass.function")] fun javaClass() : Class = null as Class -public inline fun synchronized(lock: Any, block: () -> R): R { +public inline fun synchronized(lock: Any, [inlineOptions(ONLY_LOCAL_RETURN)] block: () -> R): R { monitorEnter(lock) try { return block() diff --git a/libraries/stdlib/src/kotlin/concurrent/Locks.kt b/libraries/stdlib/src/kotlin/concurrent/Locks.kt index 93db166b0dd..f7bc5914cf3 100644 --- a/libraries/stdlib/src/kotlin/concurrent/Locks.kt +++ b/libraries/stdlib/src/kotlin/concurrent/Locks.kt @@ -5,12 +5,13 @@ import java.util.concurrent.locks.ReadWriteLock import java.util.concurrent.locks.ReentrantReadWriteLock import java.util.concurrent.locks.ReentrantReadWriteLock.ReadLock import java.util.concurrent.CountDownLatch +import kotlin.InlineOption.ONLY_LOCAL_RETURN /** Executes given calculation under lock Returns result of the calculation */ -public inline fun Lock.withLock(action: ()->T) : T { +public inline fun Lock.withLock([inlineOptions(ONLY_LOCAL_RETURN)] action: () -> T): T { lock() try { return action() @@ -24,7 +25,7 @@ public inline fun Lock.withLock(action: ()->T) : T { Executes given calculation under read lock Returns result of the calculation */ -public inline fun ReentrantReadWriteLock.read(action: ()->T) : T { +public inline fun ReentrantReadWriteLock.read([inlineOptions(ONLY_LOCAL_RETURN)] action: () -> T): T { val rl = readLock() rl.lock() try { @@ -41,7 +42,7 @@ The method does upgrade from read to write lock if needed If such write has been initiated by checking some condition, the condition must be rechecked inside the action to avoid possible races Returns result of the calculation */ -public inline fun ReentrantReadWriteLock.write(action: ()->T) : T { +public inline fun ReentrantReadWriteLock.write([inlineOptions(ONLY_LOCAL_RETURN)] action: () -> T): T { val rl = readLock() val readCount = if (getWriteHoldCount() == 0) getReadHoldCount() else 0 @@ -62,7 +63,7 @@ public inline fun ReentrantReadWriteLock.write(action: ()->T) : T { Execute given calculation and await for CountDownLatch Returns result of the calculation */ -fun Int.latch(op: CountDownLatch.() -> T) : T { +fun Int.latch(op: CountDownLatch.() -> T) : T { val cdl = CountDownLatch(this) val res = cdl.op() cdl.await() diff --git a/libraries/stdlib/src/kotlin/io/Closable.kt b/libraries/stdlib/src/kotlin/io/Closable.kt index eab1f333822..20cdffb9d65 100644 --- a/libraries/stdlib/src/kotlin/io/Closable.kt +++ b/libraries/stdlib/src/kotlin/io/Closable.kt @@ -1,9 +1,10 @@ package kotlin import java.io.Closeable +import kotlin.InlineOption.ONLY_LOCAL_RETURN /** Uses the given resource then closes it down correctly whether an exception is thrown or not */ -public inline fun T.use(block: (T) -> R): R { +public inline fun T.use([inlineOptions(ONLY_LOCAL_RETURN)] block: (T) -> R): R { var closed = false try { return block(this) diff --git a/libraries/stdlib/src/kotlin/io/JIO.kt b/libraries/stdlib/src/kotlin/io/JIO.kt index 2136380db87..42bb6d35618 100644 --- a/libraries/stdlib/src/kotlin/io/JIO.kt +++ b/libraries/stdlib/src/kotlin/io/JIO.kt @@ -4,6 +4,7 @@ import java.io.* import java.nio.charset.* import java.util.NoSuchElementException import java.net.URL +import kotlin.InlineOption.ONLY_LOCAL_RETURN /** * Returns the default buffer size when working with buffered streams @@ -198,7 +199,7 @@ public fun Writer.buffered(bufferSize: Int = defaultBufferSize): BufferedWriter */ public fun Reader.forEachLine(block: (String) -> Unit): Unit = useLines { lines -> lines.forEach(block) } -public inline fun Reader.useLines(block: (Stream) -> T): T = +public inline fun Reader.useLines([inlineOptions(ONLY_LOCAL_RETURN)] block: (Stream) -> T): T = this.buffered().use { block(it.lines()) } /**