From 7559d3095f2aa532968fb037e2e1c46e2765dc9d Mon Sep 17 00:00:00 2001 From: Tamas Vajk Date: Fri, 21 Oct 2022 08:38:30 +0200 Subject: [PATCH] Revert "Kotlin: Exclude captured variables from constant loop condition check" This reverts commit 3e476f96bd12e0821b401f20f6aa8dffd96306ca. --- java/ql/lib/semmle/code/java/Variable.qll | 12 ---------- .../Termination/ConstantLoopCondition.ql | 5 +---- .../variables/variableAccesses.expected | 5 ----- .../variables/variables.expected | 22 ------------------- .../library-tests/variables/variables.kt | 20 +---------------- .../library-tests/variables/variables.ql | 6 ----- .../query-tests/ConstantLoopCondition/A.kt | 14 ------------ .../ConstantLoopCondition.expected | 2 +- 8 files changed, 3 insertions(+), 83 deletions(-) diff --git a/java/ql/lib/semmle/code/java/Variable.qll b/java/ql/lib/semmle/code/java/Variable.qll index 219d4894827..7be49acf546 100644 --- a/java/ql/lib/semmle/code/java/Variable.qll +++ b/java/ql/lib/semmle/code/java/Variable.qll @@ -61,18 +61,6 @@ class LocalVariableDecl extends @localvar, LocalScopeVariable { override Expr getInitializer() { result = this.getDeclExpr().getInit() } override string getAPrimaryQlClass() { result = "LocalVariableDecl" } - - /** Holds if this non-final variable is captured by a nested callable. */ - predicate isCaptured() { - not this.(Modifiable).isFinal() and exists(this.getACapturingCallable()) - } - - /** Gets a callable that captures this non-final variable, if any. */ - Callable getACapturingCallable() { - not this.(Modifiable).isFinal() and - result = this.getAnAccess().getEnclosingCallable() and - result != this.getCallable() - } } /** A formal parameter of a callable. */ diff --git a/java/ql/src/Likely Bugs/Termination/ConstantLoopCondition.ql b/java/ql/src/Likely Bugs/Termination/ConstantLoopCondition.ql index 7045687826c..0b3c1256050 100644 --- a/java/ql/src/Likely Bugs/Termination/ConstantLoopCondition.ql +++ b/java/ql/src/Likely Bugs/Termination/ConstantLoopCondition.ql @@ -84,9 +84,6 @@ where not fa.getField().isFinal() and fa.getParent*() = cond ) and - not exists(ArrayAccess aa | aa.getParent*() = cond) and - not exists(RValue use | - use.getParent*() = cond and use.getVariable().(LocalVariableDecl).isCaptured() - ) + not exists(ArrayAccess aa | aa.getParent*() = cond) select cond, "$@ might not terminate, as this loop condition is constant within the loop.", loop, "Loop" diff --git a/java/ql/test/kotlin/library-tests/variables/variableAccesses.expected b/java/ql/test/kotlin/library-tests/variables/variableAccesses.expected index d535a8d4473..b29bc063a2b 100644 --- a/java/ql/test/kotlin/library-tests/variables/variableAccesses.expected +++ b/java/ql/test/kotlin/library-tests/variables/variableAccesses.expected @@ -14,11 +14,6 @@ varAcc | variables.kt:30:9:30:9 | this | | variables.kt:32:9:32:12 | this | | variables.kt:33:9:33:12 | this | -| variables.kt:52:31:52:31 | f | -| variables.kt:58:17:58:17 | c | -| variables.kt:59:17:59:17 | d | -| variables.kt:65:17:65:17 | e | -| variables.kt:66:17:66:17 | f | extensionReceiverAcc | variables.kt:29:9:29:9 | this | | variables.kt:30:9:30:9 | this | diff --git a/java/ql/test/kotlin/library-tests/variables/variables.expected b/java/ql/test/kotlin/library-tests/variables/variables.expected index cb23cd4bb41..c4aaa4a1620 100644 --- a/java/ql/test/kotlin/library-tests/variables/variables.expected +++ b/java/ql/test/kotlin/library-tests/variables/variables.expected @@ -2,29 +2,12 @@ isFinal | variables.kt:6:9:6:26 | int local1 | final | | variables.kt:8:9:8:26 | int local2 | non-final | | variables.kt:10:9:10:26 | int local3 | final | -| variables.kt:55:5:55:16 | boolean c | non-final | -| variables.kt:56:5:56:16 | boolean d | final | -| variables.kt:62:5:62:16 | boolean e | non-final | -| variables.kt:63:5:63:16 | boolean f | final | compileTimeConstant | variables.kt:3:5:3:21 | prop | | variables.kt:3:5:3:21 | this.prop | | variables.kt:7:17:7:22 | local1 | | variables.kt:15:1:15:21 | VariablesKt.topLevel | | variables.kt:15:1:15:21 | VariablesKt.topLevel | -| variables.kt:59:17:59:17 | d | -| variables.kt:66:17:66:17 | f | -isCaptured -| variables.kt:6:9:6:26 | int local1 | not captured | -| variables.kt:8:9:8:26 | int local2 | not captured | -| variables.kt:10:9:10:26 | int local3 | not captured | -| variables.kt:55:5:55:16 | boolean c | captured | -| variables.kt:56:5:56:16 | boolean d | not captured | -| variables.kt:62:5:62:16 | boolean e | captured | -| variables.kt:63:5:63:16 | boolean f | not captured | -varCaptured -| variables.kt:55:5:55:16 | boolean c | variables.kt:57:9:60:5 | invoke | -| variables.kt:62:5:62:16 | boolean e | variables.kt:64:5:67:5 | fn2 | #select | variables.kt:3:5:3:21 | prop | int | variables.kt:3:21:3:21 | 1 | | variables.kt:5:20:5:29 | param | int | file://:0:0:0:0 | | @@ -35,8 +18,3 @@ varCaptured | variables.kt:21:11:21:18 | o | C1 | file://:0:0:0:0 | | | variables.kt:21:11:21:18 | o | C1 | variables.kt:21:11:21:18 | o | | variables.kt:28:9:28:10 | | C1 | file://:0:0:0:0 | | -| variables.kt:52:9:52:26 | f | Function0 | file://:0:0:0:0 | | -| variables.kt:55:5:55:16 | boolean c | boolean | variables.kt:55:13:55:16 | true | -| variables.kt:56:5:56:16 | boolean d | boolean | variables.kt:56:13:56:16 | true | -| variables.kt:62:5:62:16 | boolean e | boolean | variables.kt:62:13:62:16 | true | -| variables.kt:63:5:63:16 | boolean f | boolean | variables.kt:63:13:63:16 | true | diff --git a/java/ql/test/kotlin/library-tests/variables/variables.kt b/java/ql/test/kotlin/library-tests/variables/variables.kt index d01a4ce65a8..f15778e9225 100644 --- a/java/ql/test/kotlin/library-tests/variables/variables.kt +++ b/java/ql/test/kotlin/library-tests/variables/variables.kt @@ -47,22 +47,4 @@ class C3 { this@C3.f0() } } -} - -fun fn0(f: Function0) = f() - -fun fn1() { - var c = true - val d = true - fn0 { - println(c) - println(d) - } - - var e = true - val f = true - fun fn2() { - println(e) - println(f) - } -} +} \ No newline at end of file diff --git a/java/ql/test/kotlin/library-tests/variables/variables.ql b/java/ql/test/kotlin/library-tests/variables/variables.ql index d6521d86d86..464674b52b2 100644 --- a/java/ql/test/kotlin/library-tests/variables/variables.ql +++ b/java/ql/test/kotlin/library-tests/variables/variables.ql @@ -43,9 +43,3 @@ query predicate isFinal(LocalVariableDecl v, string isFinal) { query predicate compileTimeConstant(CompileTimeConstantExpr e) { exists(Variable v | v.getAnAccess() = e) } - -query predicate isCaptured(LocalVariableDecl v, string captured) { - if v.isCaptured() then captured = "captured" else captured = "not captured" -} - -query predicate varCaptured(LocalVariableDecl v, Callable c) { v.getACapturingCallable() = c } diff --git a/java/ql/test/kotlin/query-tests/ConstantLoopCondition/A.kt b/java/ql/test/kotlin/query-tests/ConstantLoopCondition/A.kt index 51177f10e2e..71173626a4d 100644 --- a/java/ql/test/kotlin/query-tests/ConstantLoopCondition/A.kt +++ b/java/ql/test/kotlin/query-tests/ConstantLoopCondition/A.kt @@ -7,18 +7,4 @@ fun fn1() { c = false } } - - var d = true - while (d) { // FALSE NEGATIVE - fn0 { - println(d) - } - } - - val e = true - while (e) { - fn0 { - println(e) - } - } } diff --git a/java/ql/test/kotlin/query-tests/ConstantLoopCondition/ConstantLoopCondition.expected b/java/ql/test/kotlin/query-tests/ConstantLoopCondition/ConstantLoopCondition.expected index 2ed534139b9..151c8051624 100644 --- a/java/ql/test/kotlin/query-tests/ConstantLoopCondition/ConstantLoopCondition.expected +++ b/java/ql/test/kotlin/query-tests/ConstantLoopCondition/ConstantLoopCondition.expected @@ -1 +1 @@ -| A.kt:19:12:19:12 | e | $@ might not terminate, as this loop condition is constant within the loop. | A.kt:19:5:23:5 | while (...) | Loop | +| A.kt:5:12:5:12 | c | $@ might not terminate, as this loop condition is constant within the loop. | A.kt:5:5:9:5 | while (...) | Loop |