🔥 ข่าวด่วน: scala/scala/pull/

SolalPirelli · 2026-03-06T12:16:54Z

~~Depends on #25444~~

How much have your relied on LLM-based tools in this contribution?

not

How was the solution tested?

existing tests

SolalPirelli · 2026-03-06T15:24:09Z

  |       Cannot extend sealed trait CanEqual in a different source file
  |
  | longer explanation available when compiling with `-explain`
+2 warnings found


see the change below for why this happens but we don't print the warnings

if it's about the stale symbol, I believe it should be fixed.

SolalPirelli · 2026-03-06T15:25:10Z

@hamzaremmal I'm interested in your thoughts on the 2nd commit.

SolalPirelli · 2026-03-10T11:47:59Z

@hamzaremmal I've removed the 2nd commit, will probably reintroduce it in a later PR simplifying Vulpix in general, but I want to get the fixes to REPL tests in first

hamzaremmal · 2026-03-11T11:28:03Z

@hamzaremmal I'm interested in your thoughts on the 2nd commit.

For the record, we discussed it over Slack.

hamzaremmal · 2026-03-11T11:29:26Z

@hamzaremmal I've removed the 2nd commit, will probably reintroduce it in a later PR simplifying Vulpix in general, but I want to get the fixes to REPL tests in first

In this case, I think your second commit had some merits. It added the com.lihaoyi properties to the classpath. I believe we should do that but just hold on the other changes. No need to query the classpath and filter it yet. (What I said here: https://github.com/scala/scala3/pull/25444/changes#r2895927959)

SolalPirelli · 2026-03-11T14:11:05Z

The com.lihaoyi additions in #25444 were merged so I'm not sure I understand what you mean

hamzaremmal · 2026-03-11T14:32:14Z

Right, sorry sorry. I lost track of what was done where. Let me check again the full context and I'll approve it if it all seems ok.

hamzaremmal · 2026-03-11T14:33:33Z

+      // We run the repl with `-Ydebug` to not use the fallback if there's a class load exception so we can notice these,
+      // but as a consequence we get debug messages; ignore the stale symbol one, whose contents include symbol IDs
+      // that depend on the exact compiler.
+      (optsLine :: buf.toList).filter(nonBlank).filter(!_.startsWith("stale symbol;"))


Mmmh, this feels like a hack, why remove an error that was effectively produced? We should either fix it if it was not supposed to be produced or keep it, but hiding it is not a good idea in my opinion.

yeah, it's preexisting, it was just hidden before because we didn't run with -Ydebug. I'm not sure if it's fixable since we are, by design, creating a symbol with the same name twice, so isn't it normal that we end up with funny things?

I don't think so. We might create the same symbol twice but during different run of the compiler, right?
Anyways, it's either we can do it then we should not have a stale symbol because the compiler knows how to handle it (it's part of it's business logic: "we can redefine symbols in the same REPL session") or we cannot do it then we should have a proper error saying "you cannot redefine this symbol during the same REPL session". This feels like it's in the middle of both possibilities; allowing you to redefine but at the same time the compiler is not very happy.

yeah that's true, I'll be less lazy and try to actually fix it

SolalPirelli · 2026-03-12T15:09:35Z

@hamzaremmal Fixed, at some cost to my sanity. Turns out we were not propagating the new State in case of errors, so the next executed line used the same index for the result value name (resX) and unsurprisingly that causes funny things to happen.

Also I deleted results.scala because it was a poor attempt at abstracting Either that no longer works once you have to sometimes also propagate the state on error.

There's plenty more that could be cleaned up in the REPL, or for starters in the compiler (why are we hardcoding behavior for the REPL in the compiler??), but tests passing will do for now.

SolalPirelli · 2026-03-12T15:10:09Z

 1 error found
 scala> while (((  foo ))) {}
-- [E006] Not Found Error: -----------------------------------------------------
+-- [E007] Type Mismatch Error: -------------------------------------------------


we define it in the previous command...

hamzaremmal · 2026-03-12T15:14:55Z

@hamzaremmal Fixed, at some cost to my sanity.

I very much understand what you are saying 😞

There's plenty more that could be cleaned up in the REPL, or for starters in the compiler (why are we hardcoding behavior for the REPL in the compiler??), but tests passing will do for now.

The REPL was until recently shipped in the same artifact as the compiler. We have just recently split it and it still needs A LOT of work to fully split the business logic.

SolalPirelli · 2026-03-12T15:16:46Z

tbh the repl should be a good use case to "validate" the compiler API... if you can't easily implement a REPL, the compiler API needs work :)

hamzaremmal · 2026-03-12T15:21:44Z

tbh the repl should be a good use case to "validate" the compiler API... if you can't easily implement a REPL, the compiler API needs work :)

Agreed!

hamzaremmal

LGTM, I just have a question I would like to know the answer to before approving.

hamzaremmal · 2026-03-12T15:16:32Z

 1 error found
 scala> while (((  foo ))) {}
-- [E006] Not Found Error: -----------------------------------------------------
+-- [E007] Type Mismatch Error: -------------------------------------------------


hamzaremmal · 2026-03-12T15:17:30Z

 1 error found
 scala> ((1, 2): Foo3.T[Int][Int][Int]): Foo3.T[Any][Int][Int]
-val res2: Foo3.T[Any][Int][Int] = (1, 2)
+val res3: Foo3.T[Any][Int][Int] = (1, 2)


So the basis of the fix is propagating the new state which makes indices increment even on errors.
As I see it, it's a fix but not the root cause probably. The context (the compiler's) and ofc the name table is still polluted. My question now is: what happens if we refer to res2 (a not found??)?

(I wouldn't enshrine this into a test though, I don't think anyone should rely on this... for now at least)

Mmmh, so this is not what we want to have obviously but the proposed fix brings it closer to what we would like to have (at least the test suite behaves correctly). I'm inclined to approve this but only if we dig further to check how we can rollback the changes in the context (compiler's Context).

Now, I find this funny because what would happen if we define a value class, say class Foo and add an expression that is broken afterwards (in the same cycle). Can we still use that same class in the next run of the REPL? Would it also show the error or do we just have something that is half working (Foo works and resX shows the error)?

these are good question and I encourage you to create an issue for them, but let's please solve one problem at a time so we don't keep PRs open forever :-)

these are good question and I encourage you to create an issue for them, but let's please solve one problem at a time so we don't keep PRs open forever :-)

We are not, I just want to understand what I'm approving.

I'm inclined to approve this but only if we dig further to check how we can rollback the changes in the context (compiler's Context).

I didn't mean here to say to do it right now, as long as we have an issue that highlights all of these problems, I'll approve the PR since it brings it closer to what we want.

@hamzaremmal ping on clicking approve? 😇

I thought you will create the issue first.

hamzaremmal · 2026-03-12T15:17:59Z

 1 error found
 scala> buf ++= xs
-val res0: mutable.ListBuffer[Int] = ListBuffer(1, 2, 3)
+val res1: mutable.ListBuffer[Int] = ListBuffer(1, 2, 3)


I imagine all of those will be the same comment I made above.

SolalPirelli force-pushed the solal/repl-insanity branch from 4cb9152 to 1232da6 Compare March 6, 2026 15:23

SolalPirelli commented Mar 6, 2026

View reviewed changes

SolalPirelli requested a review from hamzaremmal March 9, 2026 08:08

SolalPirelli force-pushed the solal/repl-insanity branch from 1232da6 to bd12473 Compare March 10, 2026 11:47

SolalPirelli marked this pull request as ready for review March 10, 2026 11:47

SolalPirelli assigned hamzaremmal Mar 10, 2026

hamzaremmal reviewed Mar 11, 2026

View reviewed changes

SolalPirelli added 3 commits March 12, 2026 14:14

Force tests to not use the fallback and fix the result

7aed075

Stop being lazy (me, not the code)

811c23f

Fixed

11a7bdc

SolalPirelli force-pushed the solal/repl-insanity branch from bd12473 to 11a7bdc Compare March 12, 2026 15:07

SolalPirelli requested a review from hamzaremmal March 12, 2026 15:09

SolalPirelli commented Mar 12, 2026

View reviewed changes

hamzaremmal reviewed Mar 12, 2026

View reviewed changes

hamzaremmal mentioned this pull request Mar 13, 2026

Improve how the REPL handles errors #25514

Closed

hamzaremmal approved these changes Mar 13, 2026

View reviewed changes

hamzaremmal merged commit 0922097 into scala:main Mar 13, 2026
64 checks passed

hamzaremmal deleted the solal/repl-insanity branch March 13, 2026 12:30

WojciechMazur added this to the 3.8.4 milestone Mar 31, 2026

Uh oh!

Conversation

SolalPirelli commented Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

How much have your relied on LLM-based tools in this contribution?

How was the solution tested?

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SolalPirelli commented Mar 6, 2026

Uh oh!

SolalPirelli commented Mar 10, 2026

Uh oh!

hamzaremmal commented Mar 11, 2026

Uh oh!

hamzaremmal commented Mar 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SolalPirelli commented Mar 11, 2026

Uh oh!

hamzaremmal commented Mar 11, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SolalPirelli commented Mar 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hamzaremmal commented Mar 12, 2026

Uh oh!

SolalPirelli commented Mar 12, 2026

Uh oh!

hamzaremmal commented Mar 12, 2026

Uh oh!

hamzaremmal left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

SolalPirelli commented Mar 6, 2026 •

edited

Loading

hamzaremmal commented Mar 11, 2026 •

edited

Loading

SolalPirelli commented Mar 12, 2026 •

edited

Loading