-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Use PPrint to handle printing of REPL output values #23849
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 21 commits
d03ac8f
690ba71
e143442
cf81680
1be063a
2562be1
96b6d03
8351dbe
4bcb2c5
85f73ce
8074f15
ce4e81d
f48a8d8
5ac49a6
10bb601
b79e803
f4b89d7
2a4a1c1
cb90ed2
a4cc58d
c7d109b
8082a5d
19fdaa1
d098ca8
236abf2
0e78df2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,3 @@ | ||
scala> val foo = "1"; foo.toInt | ||
val foo: String = 1 | ||
val foo: String = "1" | ||
val res0: Int = 1 |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -675,6 +675,94 @@ object Build { | |
recur(lines) | ||
} | ||
|
||
val shadedSourceGenerator = (Compile / sourceGenerators) += Def.task { | ||
val downloads = Seq( | ||
"https://repo1.maven.org/maven2/com/lihaoyi/pprint_3/0.9.3/pprint_3-0.9.3-sources.jar", | ||
"https://repo1.maven.org/maven2/com/lihaoyi/fansi_3/0.5.1/fansi_3-0.5.1-sources.jar", | ||
"https://repo1.maven.org/maven2/com/lihaoyi/sourcecode_3/0.4.3-M5/sourcecode_3-0.4.3-M5-sources.jar", | ||
) | ||
val dest = ((Compile / sourceManaged).value / "downloaded").toPath | ||
if (Files.exists(dest)) { | ||
Files.walk(dest) | ||
.sorted(java.util.Comparator.reverseOrder()) // delete children before parents | ||
.forEach(p => Files.delete(p)); | ||
} | ||
Files.createDirectories(dest) | ||
|
||
for(url <- downloads) { | ||
import java.io._ | ||
import java.net.{HttpURLConnection, URL} | ||
import java.nio.file._ | ||
import java.nio.file.attribute.FileTime | ||
import java.util.zip.{ZipEntry, ZipInputStream} | ||
|
||
val conn = new URL(url).openConnection().asInstanceOf[HttpURLConnection] | ||
conn.setInstanceFollowRedirects(true) | ||
conn.setConnectTimeout(15000) | ||
conn.setReadTimeout(60000) | ||
conn.setRequestMethod("GET") | ||
|
||
var in: InputStream = null | ||
var zis: ZipInputStream = null | ||
try { | ||
in = new BufferedInputStream(conn.getInputStream) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If @hamzaremmal is bumping the required version to Java 17, I'm hoping to delete al of this and it'll collapse down to a single line |
||
zis = new ZipInputStream(in) | ||
|
||
var entry: ZipEntry = zis.getNextEntry | ||
val buffer = new Array[Byte](8192) | ||
|
||
while (entry != null) { | ||
val target = dest.resolve(entry.getName).normalize() | ||
if (entry.isDirectory) Files.createDirectories(target) | ||
else { | ||
Files.createDirectories(target.getParent) | ||
var out: OutputStream = null | ||
try { | ||
out = new BufferedOutputStream(Files.newOutputStream(target, StandardOpenOption.CREATE, StandardOpenOption.TRUNCATE_EXISTING)) | ||
var n = zis.read(buffer) | ||
while (n != -1) { | ||
out.write(buffer, 0, n) | ||
n = zis.read(buffer) | ||
} | ||
} finally if (out != null) out.close() | ||
} | ||
|
||
zis.closeEntry() | ||
entry = zis.getNextEntry | ||
} | ||
} finally { | ||
if (zis != null) zis.close() | ||
if (in != null) in.close() | ||
conn.disconnect() | ||
} | ||
} | ||
|
||
import collection.JavaConverters._ | ||
Files.walk(dest) | ||
.filter(p => p.toString().endsWith(".scala")) | ||
.map[java.io.File] { (file: java.nio.file.Path) => | ||
val text = new String(Files.readAllBytes(file.path), java.nio.charset.StandardCharsets.UTF_8) | ||
if (!file.getFileName().toString().equals("CollectionName.scala")) Files.write( | ||
file, | ||
("package dotty.shaded\n" + | ||
text | ||
.replace("import scala", "import _root_.scala") | ||
.replace(" scala.collection.", " _root_.scala.collection.") | ||
.replace("_root_.pprint", "_root_.dotty.shaded.pprint") | ||
.replace("_root_.fansi", "_root_.dotty.shaded.fansi") | ||
.replace("def apply(c: Char): Trie[T]", "def apply(c: Char): Trie[T] | Null") | ||
.replace("var head: Iterator[T] = null", "var head: Iterator[T] | Null = null") | ||
.replace("if (head != null && head.hasNext) true", "if (head != null && head.nn.hasNext) true") | ||
.replace("head.next()", "head.nn.next()") | ||
.replace("abstract class Walker", "@scala.annotation.nowarn abstract class Walker") | ||
.replace("object TPrintLowPri", "@scala.annotation.nowarn object TPrintLowPri")).getBytes | ||
) | ||
file.toFile | ||
|
||
} | ||
.collect(java.util.stream.Collectors.toList()).asScala.toSeq | ||
|
||
}.taskValue | ||
// Settings shared between scala3-compiler and scala3-compiler-bootstrapped | ||
lazy val commonDottyCompilerSettings = Seq( | ||
// Note: bench/profiles/projects.yml should be updated accordingly. | ||
|
@@ -721,6 +809,8 @@ object Build { | |
("io.get-coursier" %% "coursier" % "2.0.16" % Test).cross(CrossVersion.for3Use2_13), | ||
), | ||
|
||
shadedSourceGenerator, | ||
|
||
// For convenience, change the baseDirectory when running the compiler | ||
Compile / forkOptions := (Compile / forkOptions).value.withWorkingDirectory((ThisBuild / baseDirectory).value), | ||
Compile / run / forkOptions := (Compile / run / forkOptions).value.withWorkingDirectory((ThisBuild / baseDirectory).value), | ||
|
@@ -2141,6 +2231,7 @@ object Build { | |
|
||
Seq(file) | ||
}.taskValue, | ||
shadedSourceGenerator, | ||
// sbt adds all the projects to scala-tool config which breaks building the scalaInstance | ||
// as a workaround, I build it manually by only adding the compiler | ||
scalaInstance := { | ||
|
@@ -2330,6 +2421,7 @@ object Build { | |
sjsSources | ||
} (Set(scalaJSIRSourcesJar)).toSeq | ||
}.taskValue, | ||
shadedSourceGenerator | ||
) | ||
|
||
// ============================================================================================== | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
...wait, is 0.4.3-M5 a stable version? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
might want to bring it to stable before we depend on it in the compiler repo 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's almost a year old, so I guess so haha. I can tag a stable version if you would like, but the contents of the sourcejar will be unchanged
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if it's become stabilised, by all means. 👍
I'd rather avoid milestone versions here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with @Gedochao. A stable version should be used here. Also, @lihaoyi what are the versioning scheme these 3 libraries follow? I'm not a fan of cloning the sources and change the package name. I prefer to just have a dependency and use the actual library (which we do for jline and will soon do for asm too).
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, others have raised concerns in the past about using Scala libraries in the compiler codebase affecting the bootstrapping process. By building from source, we treat it effectively as Dotty's own source files, removing any divergence in the code paths: they are treated identically to scala3's own sources. If scala3 can compile itself, it should be able to compile these sources without issue
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should indeed compile these from sources. We can depend on binaries for Java libraries (hence jline and asm are fine), but not for Scala libraries.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you explain why? I thought Scala is maintaining backwards binary/tasty compatibility. Doesn't that mean we shohld always be able to depend on older scala 3 jars in the scala3 compiler regardless of how kuch bootstrapping we do?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because circular dependencies are evil. They're not too evil across time (Av2 -> Bv1 -> Av1), but they're still difficult to reason about.
And even though Scala 3 will forever be backward compat, an eventual Scala 4 wouldn't. We shouldn't paint our build into a corner. Scala 2 tried this several times over its lifetime, and rolled back every time. It's a massive pain every time it happens. There would need to be a huge upside to depending on a binary for that to be offset.