Skip to content
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

java.sql.Driver missing from classpath in macro in presentation compiler #20560

Closed
kasiaMarek opened this issue Jun 12, 2024 · 17 comments · Fixed by scalameta/metals#7002
Closed
Labels
area:inline area:presentation-compiler Related to the presentation compiler module used by Metals and possibly other tools compat:java itype:bug

Comments

@kasiaMarek
Copy link
Contributor

kasiaMarek commented Jun 12, 2024

Original issue: scalameta/metals#6494

Compiler version

3.4.2

Minimized code

//> using scala "3.4.2"
//> using dep "com.h2database:h2:2.2.224"

import scala.quoted._

class LoadStuff extends Selectable:
  def selectDynamic(name: String): Any = name

object LoadStuff:
  transparent inline def make = ${ makeImpl }
   
  private def makeImpl(using Quotes): Expr[Any] =
    import quotes.reflect.*

    Class.forName("org.h2.Driver") // problematic line

    val refinement = Refinement(TypeRepr.of[LoadStuff], "name", TypeRepr.of[String])

    refinement.asType match
      case '[refined] => '{ LoadStuff().asInstanceOf[refined] }

Output

Interactive compiler reports error diagnostics:

java.lang.ClassNotFoundException: java.sql.Driver
        at java.base/java.net.URLClassLoader.findClass(URLClassLoader.java:445)
        at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:593)
        at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:526)
        at java.base/java.lang.Class.forName0(Native Method)
        at java.base/java.lang.Class.forName(Class.java:421)
        at java.base/java.lang.Class.forName(Class.java:412)
        at a.LoadStuff$.makeImpl(LoadStuff.scala:13)
        at a.LoadStuff$.inline$makeImpl(LoadStuff.scala:10)

Expectation

Should compile fine and diagnostics should be empty.

@kasiaMarek kasiaMarek added itype:bug stat:needs triage Every issue needs to have an "area" and "itype" label area:presentation-compiler Related to the presentation compiler module used by Metals and possibly other tools labels Jun 12, 2024
@Gedochao Gedochao added compat:java area:inline and removed stat:needs triage Every issue needs to have an "area" and "itype" label labels Jun 12, 2024
@ncreep
Copy link

ncreep commented Jun 16, 2024

Hi,

Just adding some further info from the original issue.

Thinking about it, this can be further minimized, as this has nothing to do with Selectable. Apparently just being a transparent inline with class loading is enough to trigger this behavior:

object LoadStuff:
  transparent inline def make: Unit = ${ makeImpl }
   
  private def makeImpl(using Quotes): Expr[Unit] =
    Class.forName("org.h2.Driver")

    '{()}

This too triggers a loss of type-information at the call-site of make.

Removing transparent brings back type information.

Thanks

@ncreep
Copy link

ncreep commented Oct 29, 2024

Hi,

I'd like to take a stab at figuring out this issue.
Can anyone please point me where to start looking in the repo?

Thanks

@jchyb
Copy link
Contributor

jchyb commented Oct 29, 2024

Hello!
Macro methods (the previously compiled .class code run on runtime) are called from the compiler/src/dotty/tools/dotc/quoted/Interpreter.scala class. There, a separately constructed classLoader is used to run that previously compiled code (the Interpreter name might be misleading, the only thing that is being interpreted is the ${ makeImpl } part, then makeImpl is run via invoking a method from a classfile). Since that class loader is constructed separately I suspect it might be the thing causing the issue. Previously we already had a problem with it in the cli, caused by the fact that calling java -cp SomeClass:compilerClasses -classPath OtherClasses replMain would not join SomeClass with OtherClasses in that class loader (but worked for non macro compilation). I do not know much about the presentation compiler, so I cannot help with that aspect unfortunately.
Making any progress here or observation would be of great help, I recall taking a stab at this some time ago but not being able to make any progress

@ncreep
Copy link

ncreep commented Oct 29, 2024

Thank you for the detailed response!

Is there a guide on how to develop and diagnose the presentation compiler with Metals?
E.g.:

  • How do I see the exception in the issue description? It doesn't seem to be visible as a regular Metals user.
  • How do I plug my local Metals setup to use a local version of the compiler?

Thanks

@tgodzik
Copy link
Contributor

tgodzik commented Oct 29, 2024

  • How do I see the exception in the issue description? It doesn't seem to be visible as a regular Metals user.

That should be visible in .metals/reports or in .metals/metals.log

  • How do I plug my local Metals setup to use a local version of the compiler?

You only have to publish locally the presentation compiler module, it should be picked up automatically.

@ncreep
Copy link

ncreep commented Oct 29, 2024

Thanks for the prompt response.

Trying to reproduce the bug now, and it's still missing type information, but also, I'm not seeing anything in the logs at all (only successful compilation).

In the original issue I reported in the Metals repository, I did get an exception visible somewhere, though I don't recall now where I got it from.

But even back then, as can be seen in the discussion there, I wasn't seeing the actual exception thrown by the presentation compiler, but rather some subsequent exception coming from Metals. It was mentioned there by Kasia, that it should be possible to get diagnostics directly from the presentation compiler. Can you please point me to where these are visible?

Thanks

@tgodzik
Copy link
Contributor

tgodzik commented Oct 30, 2024

Curious, I can't seem to reproduce either:

Screencast.from.2024-10-30.10-55-34.mp4

The only issue I see is that no hover information is shown in some places, but not actual exception. We could try to fix that and use Hover tests for that.

@kasiaMarek
Copy link
Contributor Author

kasiaMarek commented Oct 30, 2024

Maybe it got somehow fixed or maybe there was some fallback in metals added, so there is no error visible. The error I posted was only visible when logging diagnostics from pc compile run, it did not surface.

@ncreep
Copy link

ncreep commented Oct 30, 2024

Although I've no idea where the exception has gone, the full issue at the usage site is still present:

  • No hover
  • No autocompletion
  • No "insert type annotation"

Which I would imagine indicates that the presentation compiler decided to quit at this point.

@ncreep
Copy link

ncreep commented Oct 30, 2024

Also, how can I enable diagnostics from the presentation compiler?

Thanks

@ncreep
Copy link

ncreep commented Oct 30, 2024

Playing a bit more with this issue. It seems that there's a definite improvement in 3.5.x compared to 3.4.2.

In 3.4.2 the whole usage file completely loses type information in Metals. In 3.5.x it's only the results of the transparent inline that lose information, the rest of the file continues to function.

Maybe best effort compilation catches and suppresses errors mid way?

@kasiaMarek
Copy link
Contributor Author

Also, how can I enable diagnostics from the presentation compiler?

If you are expecting Metals to display those diagnostics it won't. You have to change the code. Basically after type checking in run on compilation unit in pc you can call diagnostics (or something similar) method, and log that information. This is as far as I remember from the top of my head, I'd have to check for details.

@ncreep
Copy link

ncreep commented Oct 30, 2024

@kasiaMarek, got it, thanks.

@mbovel
Copy link
Member

mbovel commented Dec 1, 2024

This issue was picked for the Scala Issue Spree of tomorrow, Monday, December 2nd. @aherlihy, @KuceraMartin, @mbovel and @jan-pieter will be working on it.

@rochala
Copy link
Contributor

rochala commented Dec 3, 2024

#22101 Confirms that the issue is actually not in compiler, but in the way how metals load presentation compiler.

@rochala rochala closed this as completed Dec 3, 2024
mbovel added a commit that referenced this issue Dec 4, 2024
This PR adds a mechanism to test the presentation compiler with snippets
using arbitrary pre-compiled files. To do so, it adds a new project
`scala3-presentation-compiler-testcases`, compiled with the bootstrapped
compiler, which is used as a dependency of
`scala3-presentation-compiler`. The resulting class path is added to the
`ideTestsDependencyClasspath` build info so that the test cases can be
used from the presentation compiler tests.

This PR also adds a test case for #20560. It shows that there is no
hover info for the resulting type of a `transparent inline` macro when
it fails to execute. However, the macro succeeds when loading the class
`java.sql.Driver`, so that still doesn't tell us what the problem is
with #20560
@mbovel
Copy link
Member

mbovel commented Dec 4, 2024

I just tried the example again with the version 1.4.1+73-c291bd28-SNAPSHOT of the Metals server, and I confirm it works! 🥳

@ncreep
Copy link

ncreep commented Dec 4, 2024

Thank you so much for fixing this!

I can confirm as well that both the example and the real code that triggered the error both work correctly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:inline area:presentation-compiler Related to the presentation compiler module used by Metals and possibly other tools compat:java itype:bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants