From c9984c3d06138bd14fcb337ccc78d50159f57f43 Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Thu, 4 Oct 2012 15:46:25 +0400 Subject: [PATCH] Generate not-null assertions on method parameters Intrinsics.checkParameterIsNotNull() gets its caller's class and method names from the stack trace to render them in an exception message. Fix codegen tests because now it's now allowed to pass null to non-null argument in tests --- .../jet/compiler/android/SpecialFiles.java | 1 + .../org/jetbrains/jet/codegen/AsmUtil.java | 25 ++++++++++++++ .../jet/codegen/FunctionCodegen.java | 2 ++ .../doGenerateParamAssertions.java | 17 ++++++++++ .../doGenerateParamAssertions.kt | 8 +++++ .../doNotGenerateParamAssertions.kt | 4 +++ .../noAssertionForPrivateMethod.kt | 4 +++ .../GenerateNotNullAssertionsTest.java | 33 ++++++++++++++++--- .../jetbrains/jet/codegen/StringsTest.java | 4 --- runtime/src/jet/runtime/Intrinsics.java | 19 ++++++++++- 10 files changed, 108 insertions(+), 9 deletions(-) create mode 100644 compiler/testData/codegen/notNullAssertions/doGenerateParamAssertions.java create mode 100644 compiler/testData/codegen/notNullAssertions/doGenerateParamAssertions.kt create mode 100644 compiler/testData/codegen/notNullAssertions/doNotGenerateParamAssertions.kt create mode 100644 compiler/testData/codegen/notNullAssertions/noAssertionForPrivateMethod.kt diff --git a/compiler/android-tests/tests/org/jetbrains/jet/compiler/android/SpecialFiles.java b/compiler/android-tests/tests/org/jetbrains/jet/compiler/android/SpecialFiles.java index b9c6acb45e4..116d1df684f 100644 --- a/compiler/android-tests/tests/org/jetbrains/jet/compiler/android/SpecialFiles.java +++ b/compiler/android-tests/tests/org/jetbrains/jet/compiler/android/SpecialFiles.java @@ -129,6 +129,7 @@ public class SpecialFiles { excludedFiles.add("doGenerateAssertions.kt"); // Multi-file + Java excludedFiles.add("doNotGenerateAssertions.kt"); // Multi-file + Java + excludedFiles.add("doGenerateParamAssertions.kt"); // Java } private SpecialFiles() { diff --git a/compiler/backend/src/org/jetbrains/jet/codegen/AsmUtil.java b/compiler/backend/src/org/jetbrains/jet/codegen/AsmUtil.java index 5c8fe2378da..ba8253c4d69 100644 --- a/compiler/backend/src/org/jetbrains/jet/codegen/AsmUtil.java +++ b/compiler/backend/src/org/jetbrains/jet/codegen/AsmUtil.java @@ -422,6 +422,31 @@ public class AsmUtil { genMethodThrow(mv, STUB_EXCEPTION, STUB_EXCEPTION_MESSAGE); } + public static void genNotNullAssertionsForParameters( + @NotNull InstructionAdapter v, + @NotNull GenerationState state, + @NotNull FunctionDescriptor descriptor, + @NotNull FrameMap frameMap + ) { + if (!state.isGenerateNotNullParamAssertions()) return; + + // Private method is not accessible from other classes, no assertions needed + if (getVisibilityAccessFlag(descriptor) == ACC_PRIVATE) return; + + for (ValueParameterDescriptor parameter : descriptor.getValueParameters()) { + JetType type = parameter.getReturnType(); + if (type == null || type.isNullable()) continue; + + int index = frameMap.getIndex(parameter); + Type asmType = state.getTypeMapper().mapReturnType(type); + if (asmType.getSort() == Type.OBJECT || asmType.getSort() == Type.ARRAY) { + v.load(index, asmType); + v.visitLdcInsn(descriptor.getName().getName()); + v.invokestatic("jet/runtime/Intrinsics", "checkParameterIsNotNull", "(Ljava/lang/Object;Ljava/lang/String;)V"); + } + } + } + public static void genNotNullAssertionForField( @NotNull InstructionAdapter v, @NotNull GenerationState state, diff --git a/compiler/backend/src/org/jetbrains/jet/codegen/FunctionCodegen.java b/compiler/backend/src/org/jetbrains/jet/codegen/FunctionCodegen.java index 77a8bbe0282..21cd79676e2 100644 --- a/compiler/backend/src/org/jetbrains/jet/codegen/FunctionCodegen.java +++ b/compiler/backend/src/org/jetbrains/jet/codegen/FunctionCodegen.java @@ -211,6 +211,8 @@ public class FunctionCodegen extends GenerationStateAware { createSharedVarsForParameters(mv, functionDescriptor, frameMap, localVariablesInfo); + genNotNullAssertionsForParameters(new InstructionAdapter(mv), state, functionDescriptor, frameMap); + ExpressionCodegen codegen = new ExpressionCodegen(mv, frameMap, asmMethod.getReturnType(), context, state); codegen.returnExpression(fun.getBodyExpression()); diff --git a/compiler/testData/codegen/notNullAssertions/doGenerateParamAssertions.java b/compiler/testData/codegen/notNullAssertions/doGenerateParamAssertions.java new file mode 100644 index 00000000000..34289912f4e --- /dev/null +++ b/compiler/testData/codegen/notNullAssertions/doGenerateParamAssertions.java @@ -0,0 +1,17 @@ +package test; + +import org.jetbrains.annotations.NotNull; + +public abstract class doGenerateParamAssertions { + + public abstract void bar(@NotNull String s); + + public static String foo(doGenerateParamAssertions a) { + try { + a.bar(null); + } catch (IllegalArgumentException e) { + return "OK"; + } + return "Fail: AssertionError expected"; + } +} diff --git a/compiler/testData/codegen/notNullAssertions/doGenerateParamAssertions.kt b/compiler/testData/codegen/notNullAssertions/doGenerateParamAssertions.kt new file mode 100644 index 00000000000..0e352285f21 --- /dev/null +++ b/compiler/testData/codegen/notNullAssertions/doGenerateParamAssertions.kt @@ -0,0 +1,8 @@ +import test.doGenerateParamAssertions as C + +class A : C() { + override fun bar(s: String) { + } +} + +fun box() = C.foo(A()) diff --git a/compiler/testData/codegen/notNullAssertions/doNotGenerateParamAssertions.kt b/compiler/testData/codegen/notNullAssertions/doNotGenerateParamAssertions.kt new file mode 100644 index 00000000000..a702af9e5bb --- /dev/null +++ b/compiler/testData/codegen/notNullAssertions/doNotGenerateParamAssertions.kt @@ -0,0 +1,4 @@ +class A { + fun foo(s: String) { + } +} diff --git a/compiler/testData/codegen/notNullAssertions/noAssertionForPrivateMethod.kt b/compiler/testData/codegen/notNullAssertions/noAssertionForPrivateMethod.kt new file mode 100644 index 00000000000..49267bc3789 --- /dev/null +++ b/compiler/testData/codegen/notNullAssertions/noAssertionForPrivateMethod.kt @@ -0,0 +1,4 @@ +class A { + private fun foo(s: String) { + } +} diff --git a/compiler/tests/org/jetbrains/jet/codegen/GenerateNotNullAssertionsTest.java b/compiler/tests/org/jetbrains/jet/codegen/GenerateNotNullAssertionsTest.java index f646452daec..bf83fc312af 100644 --- a/compiler/tests/org/jetbrains/jet/codegen/GenerateNotNullAssertionsTest.java +++ b/compiler/tests/org/jetbrains/jet/codegen/GenerateNotNullAssertionsTest.java @@ -41,11 +41,12 @@ public class GenerateNotNullAssertionsTest extends CodegenTestCase { super.setUp(); } - private void setUpEnvironment(boolean generateAssertions, File... extraClassPath) { + private void setUpEnvironment(boolean generateAssertions, boolean generateParamAssertions, File... extraClassPath) { CompilerConfiguration configuration = CompileCompilerDependenciesTest.compilerConfigurationForTests( ConfigurationKind.JDK_ONLY, TestJdkKind.MOCK_JDK, extraClassPath); configuration.put(JVMConfigurationKeys.GENERATE_NOT_NULL_ASSERTIONS, generateAssertions); + configuration.put(JVMConfigurationKeys.GENERATE_NOT_NULL_PARAMETER_ASSERTIONS, generateParamAssertions); myEnvironment = new JetCoreEnvironment(getTestRootDisposable(), configuration); } @@ -53,7 +54,7 @@ public class GenerateNotNullAssertionsTest extends CodegenTestCase { private void doTestGenerateAssertions(boolean generateAssertions, String ktFile) throws Exception { File javaClassesTempDirectory = compileJava("notNullAssertions/A.java"); - setUpEnvironment(generateAssertions, javaClassesTempDirectory); + setUpEnvironment(generateAssertions, false, javaClassesTempDirectory); blackBoxMultiFile("OK", false, "notNullAssertions/AssertionChecker.kt", ktFile); } @@ -67,7 +68,7 @@ public class GenerateNotNullAssertionsTest extends CodegenTestCase { } public void testNoAssertionsForKotlinFromSource() throws Exception { - setUpEnvironment(true); + setUpEnvironment(true, false); loadFiles("notNullAssertions/noAssertionsForKotlin.kt", "notNullAssertions/noAssertionsForKotlinMain.kt"); @@ -83,13 +84,37 @@ public class GenerateNotNullAssertionsTest extends CodegenTestCase { File compiledDirectory = new File(FileUtil.getTempDirectory(), "kotlin-classes"); CompileEnvironmentUtil.writeToOutputDirectory(state.getFactory(), compiledDirectory); - setUpEnvironment(true, compiledDirectory); + setUpEnvironment(true, false, compiledDirectory); loadFile("notNullAssertions/noAssertionsForKotlinMain.kt"); assertNoIntrinsicsMethodIsCalled("namespace"); } + public void testGenerateParamAssertions() throws Exception { + File javaClassesTempDirectory = compileJava("notNullAssertions/doGenerateParamAssertions.java"); + + setUpEnvironment(false, true, javaClassesTempDirectory); + + blackBoxFile("notNullAssertions/doGenerateParamAssertions.kt"); + } + + public void testDoNotGenerateParamAssertions() throws Exception { + setUpEnvironment(false, false); + + loadFile("notNullAssertions/doNotGenerateParamAssertions.kt"); + + assertNoIntrinsicsMethodIsCalled("A"); + } + + public void testNoParamAssertionForPrivateMethod() throws Exception { + setUpEnvironment(false, true); + + loadFile("notNullAssertions/noAssertionForPrivateMethod.kt"); + + assertNoIntrinsicsMethodIsCalled("A"); + } + private void assertNoIntrinsicsMethodIsCalled(String className) { ClassFileFactory classes = generateClassesInFile(); ClassReader reader = new ClassReader(classes.asBytes(className + ".class")); diff --git a/compiler/tests/org/jetbrains/jet/codegen/StringsTest.java b/compiler/tests/org/jetbrains/jet/codegen/StringsTest.java index b49dbe8e46a..b1bb2b3f42e 100644 --- a/compiler/tests/org/jetbrains/jet/codegen/StringsTest.java +++ b/compiler/tests/org/jetbrains/jet/codegen/StringsTest.java @@ -38,8 +38,6 @@ public class StringsTest extends CodegenTestCase { // System.out.println(generateToText()); Method foo = generateFunction(); assertEquals("something", foo.invoke(null, "something")); - assertEquals("null", foo.invoke(null, new Object[]{null})); - } public void testNullableAnyToString () throws InvocationTargetException, IllegalAccessException { @@ -71,9 +69,7 @@ public class StringsTest extends CodegenTestCase { // System.out.println(text); Method foo = generateFunction(); assertEquals("something239120", foo.invoke(null, "something", 239)); - assertEquals("null239120", foo.invoke(null, null, 239)); assertEquals("239null120", foo.invoke(null, "239", null)); - assertEquals("nullnull120", foo.invoke(null, null, null)); } diff --git a/runtime/src/jet/runtime/Intrinsics.java b/runtime/src/jet/runtime/Intrinsics.java index 9478d053d0e..dedb273c1a5 100644 --- a/runtime/src/jet/runtime/Intrinsics.java +++ b/runtime/src/jet/runtime/Intrinsics.java @@ -51,6 +51,23 @@ public class Intrinsics { } } + public static void checkParameterIsNotNull(Object value, String paramName) { + if (value == null) { + StackTraceElement[] stackTraceElements = Thread.currentThread().getStackTrace(); + + // #0 is Thread.getStackTrace(), #1 is Intrinsics.checkParameterIsNotNull, #2 is our caller + StackTraceElement caller = stackTraceElements[2]; + String className = caller.getClassName(); + String methodName = caller.getMethodName(); + + IllegalArgumentException exception = + new IllegalArgumentException("Parameter specified as non-null contains null: " + + "method " + className + "." + methodName + + ", parameter " + paramName); + throw sanitizeStackTrace(exception); + } + } + public static Class getJavaClass(T self) { return (Class) self.getClass(); } @@ -74,7 +91,7 @@ public class Intrinsics { } private static final Set METHOD_NAMES_TO_SKIP = new HashSet(Arrays.asList( - "throwNpe", "checkReturnedValueIsNotNull", "checkFieldIsNotNull" + "throwNpe", "checkReturnedValueIsNotNull", "checkFieldIsNotNull", "checkParameterIsNotNull" )); private static T sanitizeStackTrace(T throwable) {