Mention if missing symbol is on the class path #10774

som-snytt · 2024-05-07T03:52:32Z

Hint if there is an object in scope, in a package, which has a class, and the class path finds it.

som-snytt · 2024-05-07T05:28:01Z

java.io.IOException: No space left on device

SethTisue · 2024-05-07T16:31:52Z

I re-ran the Jenkins build, now that the community build runs that were using all the disk have finished.

I wonder sometimes if we should have a dedicated worker behemoth just for PR validation, separate from the behemoths used for the community build.

som-snytt · 2024-05-07T22:06:14Z

Needs refinement, as it does add verbiage for incremental compilation error.

[error] /home/amarki/snips/test-12289/src/main/scala/c.scala:4:17: not found: type T
[error] A definition on the class path may be hidden by a source artifact. (Companions must be defined together.)

because (as will surprise no one) the class is still there, i.e., previous artifacts are not cleaned first.

./target/scala-2.13/classes/p/Main.class
./target/scala-2.13/classes/p/T.class
./target/scala-2.13/classes/p/Main$.class
./target/scala-2.13/classes/p/C.class
./target/scala-2.13/classes/p/Main$delayedInit$body.class

som-snytt · 2024-05-08T01:10:36Z

The clever test is tweaked to allow -d.

The output check in typer may require tweaking: it uses single output only, and compares the path strings to mean "includes".

lrytz

It seems a helpful addition. Given

package p {
  object T
  class C extends T
}

and

package p {
  trait T
}

I assume IntelliJ would happily navigate from extends T to the definition of trait T, which makes it harder to figure why compilation fails.

lrytz · 2024-05-08T06:40:24Z

test/files/neg/t12289/c_2.scala

@@ -0,0 +1,6 @@
+//> using options -d /tmp


Maybe make it a DirectTest and use testOutput.path?

How about -Ystop-before:jvm. But a test for programmatic outputDirs.outputs would have to be DirectTest.

-Yskip:jvm is somehow cooler. Either works. Could stop earlier, as it's just a backstop.

lrytz · 2024-05-08T06:44:00Z

src/compiler/scala/tools/nsc/typechecker/Typers.scala

-              case NoSymbol => issue(SymbolNotFoundError(tree, name, context.owner, startContext, mode.in(all = PATTERNmode, none = APPSELmode | TYPEPATmode)))
+              case NoSymbol =>
+                def hasOutput = settings.outputDirs.getSingleOutput.isDefined || settings.outputDirs.outputs.nonEmpty
+                val hidden = !unit.isJava && name.isTypeName && hasOutput && {


The issue doesn't manifest the other way around, if name.isTermName?

I was too lazy to check after getting a first cut. Also, I considered it more likely to add an object as factory for an existing class.

lrytz · 2024-05-08T06:48:57Z

src/compiler/scala/tools/nsc/typechecker/Typers.scala

+                val hidden = !unit.isJava && name.isTypeName && hasOutput && {
+                  startContext.lookupSymbol(name.toTermName, _ => true) match {
+                    case LookupSucceeded(qualifier, symbol) if symbol.owner.hasPackageFlag && symbol.sourceFile != null =>
+                      symbol.owner.info.decls.lookupAll(name).toList match {


This I don't immediately understand; lookup for name failed (we're about to issue an error), but lookupSymbol(name.toTermName).owner.info.decls.lookupAll(name) gives a result. How is that?

The package is loaded from class path first, which creates symbols of existing class + its module & module class. Then compilation of source takes over the existing module symbol, which is entered into scope.

It's fishy that the package class has a set of members but not all are in scope. Previously I've read a comment about stale symbols from previous run; symbol history, like human history, is a story of wreckage and detritus. Somehow I had never fully considered the state of the environment during incremental compilation.

lrytz · 2024-05-08T06:52:04Z

src/compiler/scala/tools/nsc/typechecker/ContextErrors.scala

-        NormalTypeError(tree, s"not found: ${decodeWithKind(name, owner)}$help")
+      def SymbolNotFoundError(tree: Tree, name: Name, owner: Symbol, inPattern: Boolean, hidden: Boolean) = {
+        val help = if (inPattern && name.isTermName) s"\nIdentifiers ${if (name.charAt(0).isUpper) "that begin with uppercase" else "enclosed in backticks"} are not pattern variables but match the value in scope." else ""
+        val path = if (hidden) s"\nA definition on the class path may be hidden by a source artifact. (Companions must be defined together.)" else ""


Maybe we can find a message that's a bit more explicit, hands-on. Connecting the dots between a "definition hidden by a source artifact" and "companions" is not easy simple.

User code may reference a class C on the class path which is shadowed because user code introduces "false friend" companion object C and its companion class C. Similarly when referring to an object C while intoducing a class C. Only add the hint if the hidden symbol is on the class path but not in a current output directory, i.e., a stale symbol from a previous compilation.

som-snytt · 2024-06-01T22:44:25Z

Followed suggestions for better message, tweak test so -d /tmp never outputs, add test for the "multiple -d" case (not sure if that is used by current tooling) and handle that. Also handle shadowing of companion class or companion module, as pointed out in the review.

I agree with the previous comment that REPL's reminder that "companions must be defined together" is not optional. This synthetic shadowing is very tricky.

dotty has same behavior:

➜  scala git:(issue/12289-module-class-shadowing) scalac -d /tmp/sandbox test/files/neg/t12289/t_1.scala
➜  scala git:(issue/12289-module-class-shadowing) scalac -cp /tmp/sandbox -d /tmp/sandbox test/files/neg/t12289/c_2.scala
-- [E006] Not Found Error: test/files/neg/t12289/c_2.scala:6:18 ----------------------------------------------------------------------
6 |  class C extends T
  |                  ^
  |                  Not found: type T
  |
  | longer explanation available when compiling with `-explain`
1 error found

FSR I don't understand yet, the test under Vulpix says

-- [E006] Not Found Error: tests/neg/i20510/c_2.scala:6:18 -------------------------------------------------------------
6 |  class C extends T // error
                  ^
                  Not found: type T - did you mean T.type?

som-snytt · 2024-06-01T23:25:16Z

If I weren't so lazy, I'd help with why the spec build fails ~~so often~~ always. That was a problem in the old days, but why is it a problem in the new days?

lrytz

I hope this will prevent a lot of headscratching! Thanks.

som-snytt · 2024-06-04T16:56:02Z

Periodic reminder to get out the xpres spa scalp massager device. It's quite soothing.

lrytz · 2024-06-04T17:07:03Z

Some soothing might be needed because this PR broke on Windows... https://github.com/scala/scala/actions/runs/9369646735/job/25794666793

+error: /tmp does not exist or is not a directory

lrytz · 2024-06-04T17:10:17Z

why the spec build fails

https://travis-ci.community/t/travis-focal-ubuntu-image-uses-now-expired-mongodb-4-4-package/14293/4

som-snytt · 2024-06-04T17:11:30Z

Well, you warned me about /tmp. I'll follow up now.

scala-jenkins added this to the 2.13.15 milestone May 7, 2024

som-snytt force-pushed the issue/12289-module-class-shadowing branch from a41bb8c to f89150a Compare May 8, 2024 01:09

som-snytt marked this pull request as ready for review May 8, 2024 01:12

lrytz reviewed May 8, 2024

View reviewed changes

som-snytt force-pushed the issue/12289-module-class-shadowing branch from f89150a to 14b7b8d Compare June 1, 2024 22:39

som-snytt mentioned this pull request Jun 1, 2024

Tell me if I try to define a companion to an existing symbol that is separately compiled scala/scala3#20510

Open

som-snytt changed the title ~~Mention if missing class is on the class path~~ Mention if missing symbol is on the class path Jun 1, 2024

som-snytt requested a review from lrytz June 3, 2024 20:38

lrytz approved these changes Jun 4, 2024

View reviewed changes

lrytz merged commit f2c97b3 into scala:2.13.x Jun 4, 2024
2 of 3 checks passed

SethTisue added the release-notes worth highlighting in next release notes label Jun 4, 2024

som-snytt deleted the issue/12289-module-class-shadowing branch June 4, 2024 16:56

som-snytt mentioned this pull request Jun 4, 2024

Avoid /tmp in tests #10788

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Mention if missing symbol is on the class path #10774

Mention if missing symbol is on the class path #10774

som-snytt commented May 7, 2024

som-snytt commented May 7, 2024

SethTisue commented May 7, 2024

som-snytt commented May 7, 2024

som-snytt commented May 8, 2024 •

edited

lrytz left a comment

lrytz May 8, 2024

som-snytt May 30, 2024

som-snytt May 30, 2024

lrytz May 8, 2024

som-snytt May 8, 2024

lrytz May 8, 2024

som-snytt May 8, 2024

lrytz May 8, 2024

som-snytt commented Jun 1, 2024 •

edited

som-snytt commented Jun 1, 2024 •

edited

lrytz left a comment

som-snytt commented Jun 4, 2024

lrytz commented Jun 4, 2024

lrytz commented Jun 4, 2024

som-snytt commented Jun 4, 2024

Mention if missing symbol is on the class path #10774

Mention if missing symbol is on the class path #10774

Conversation

som-snytt commented May 7, 2024

som-snytt commented May 7, 2024

SethTisue commented May 7, 2024

som-snytt commented May 7, 2024

som-snytt commented May 8, 2024 • edited

lrytz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

som-snytt commented Jun 1, 2024 • edited

som-snytt commented Jun 1, 2024 • edited

lrytz left a comment

Choose a reason for hiding this comment

som-snytt commented Jun 4, 2024

lrytz commented Jun 4, 2024

lrytz commented Jun 4, 2024

som-snytt commented Jun 4, 2024

som-snytt commented May 8, 2024 •

edited

som-snytt commented Jun 1, 2024 •

edited

som-snytt commented Jun 1, 2024 •

edited