Skip to content

Commit 65b4326

Browse files
authored
ExtractDependencies: more robust cycle check (#13583)
Checking for seen types alone wasn't robust enough since the cycle might involve a type`=:=` but not `eq` to a type we've already seen. Fixes #13575.
2 parents abcc58b + e60d1dd commit 65b4326

File tree

6 files changed

+71
-5
lines changed

6 files changed

+71
-5
lines changed

compiler/src/dotty/tools/dotc/sbt/ExtractDependencies.scala

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -453,18 +453,20 @@ private class ExtractDependenciesCollector extends tpd.TreeTraverser { thisTreeT
453453
private abstract class TypeDependencyTraverser(using Context) extends TypeTraverser() {
454454
protected def addDependency(symbol: Symbol): Unit
455455

456-
val seen = new mutable.HashSet[Type]
456+
// Avoid cycles by remembering both the types (testcase:
457+
// tests/run/enum-values.scala) and the symbols of named types (testcase:
458+
// tests/pos-java-interop/i13575) we've seen before.
459+
val seen = new mutable.HashSet[Symbol | Type]
457460
def traverse(tp: Type): Unit = if (!seen.contains(tp)) {
458461
seen += tp
459462
tp match {
460463
case tp: NamedType =>
461464
val sym = tp.symbol
462-
if (!sym.is(Package)) {
465+
if !seen.contains(sym) && !sym.is(Package) then
466+
seen += sym
463467
addDependency(sym)
464-
if (!sym.isClass)
465-
traverse(tp.info)
468+
if !sym.isClass then traverse(tp.info)
466469
traverse(tp.prefix)
467-
}
468470
case tp: ThisType =>
469471
traverse(tp.underlying)
470472
case tp: ConstantType =>
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
package com.lamoroso.example;
2+
3+
import java.util.Collections;
4+
import java.util.List;
5+
6+
public abstract class Builder<T extends Builder<T, R>, R> {
7+
8+
private List<String> pools;
9+
10+
public Builder<T, R> withPool(String... pools) {
11+
Collections.addAll(this.pools, pools);
12+
return this;
13+
}
14+
15+
public Builder<T, R> build(){return null;}
16+
}
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
package com.lamoroso.example;
2+
3+
public class Client {
4+
5+
public static Builder<?, ?> builder() {
6+
return null;
7+
}
8+
9+
}
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
package com.lamoroso.example;
2+
3+
public class RestClient {
4+
5+
private Object instance;
6+
7+
public RestClient(Object instance) {
8+
this.instance = instance;
9+
}
10+
11+
public static RestClientBuilder<?,?> builder() {
12+
return new RestClientBuilder();
13+
}
14+
15+
}
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
package com.lamoroso.example;
2+
3+
public class RestClientBuilder<T extends Builder<T, R>, R> {
4+
5+
private Builder<?, ?> wrappedBuilder;
6+
7+
protected RestClientBuilder() {
8+
this.wrappedBuilder = Client.builder();
9+
}
10+
11+
public RestClientBuilder<T, R> withPool(String... pools) {
12+
this.wrappedBuilder.withPool(pools);
13+
return this;
14+
}
15+
16+
public RestClient build() {
17+
return new RestClient(wrappedBuilder.build());
18+
}
19+
}
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
package com.lamoroso.example
2+
3+
object ScalaApp extends App {
4+
RestClient.builder().withPool("hello").build()
5+
}

0 commit comments

Comments
 (0)