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.

Sign up for GitHub

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

Jump to bottom

WIP: feature: Support NIR file decoding #5588

Closed
wants to merge 1 commit into from
Closed

WIP: feature: Support NIR file decoding #5588

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 61 additions & 4 deletions metals/src/main/scala/scala/meta/internal/metals/FileDecoderProvider.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,9 @@ final class FileDecoderProvider(
*
* build target:
* metalsDecode:file:///workspacePath/buildTargetName.metals-buildtarget
*
* native intermediate representation (NIR):
* metalsDecode:file:///somePath/someFile.scala.nir
*/
def decodedFileContents(uriAsStr: String): Future[DecoderResponse] = {
Try(URI.create(URIEncoderDecoder.encode(uriAsStr))) match {
Expand Down Expand Up @@ -209,10 +212,7 @@ final class FileDecoderProvider(
uri: URI
): Future[DecoderResponse] = {
val supportedExtensions = Set(
"javap",
"javap-verbose",
"tasty-decoded",
"cfr",
"javap", "javap-verbose", "tasty-decoded", "cfr", "nir",
) ++ semanticdbExtensions
val additionalExtension = uri.toString().split('.').toList.last
if (supportedExtensions(additionalExtension)) {
Expand All @@ -226,6 +226,12 @@ final class FileDecoderProvider(
case "javap-verbose" =>
decodeJavaOrScalaOrClass(path, decodeJavapFromClassFile(true))
case "cfr" => decodeJavaOrScalaOrClass(path, decodeCFRFromClassFile)
case "nir" =>
selectClassFromScalaFileAndDecode(
path.toURI,
path,
ClassFinderGranularity.NIR,
)(p => decodeNIR(p.path))
case "tasty-decoded" => decodeTasty(path)
case "semanticdb-compact" =>
Future.successful(
Expand Down Expand Up @@ -568,6 +574,57 @@ final class FileDecoderProvider(
}
}

private def decodeNIR(
path: AbsolutePath
): Future[DecoderResponse] = {
val scalaNativeCLIDependency =
Dependency.of("org.scala-native", "scala-native-cli_2.13", "0.4.10")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Latest is 0.4.14. This will have to stay pretty up-to-date b/c NIR is continually evolving, I wonder if there is a better way we can manage this version.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess my concern was having metals depend on the scala-native-cli (or nir) libraries directly. If that's not a concern then this dynamic download is probably not required.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, actually I think a dynamic download is better because it would allow the scala-native-cli version to be bumped to latest without requiring a new release of metals.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious if I can:

  1. add this as an optional dep so steward can do the update PRs.
  2. Pass the version in with build info
  3. Default a config to that version
  4. Use that condid here

A few steps but would hopefully balance devex of this code with usability

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we not check the classpath which version of native is being used?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would think so? If there is a way to access the native build target then I presume that can be inspected to determine the native version.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't have a separate build target data for native (could be done), but we do have a list of jars that is being used, so potentially we could scan it for a particular native jar and check the pom.xml or try to figure out native version from the name (not the greatest, but we can fallback to the newest known version if the approach fails)

val scalaNativePMain = "scala.scalanative.cli.ScalaNativeP"

// the class finder gets us the .nir under the java built target. How to get the proper native one?
// the replaceAll below assumes a particular layout.
val parent = path.parent
val args = List(
"--verbose",
"--from-path",
path.toString().replaceAll(".jvm", ".native"),
)
val sbOut = new StringBuilder()
val sbErr = new StringBuilder()
try {
shellRunner
.runJava(
scalaNativeCLIDependency,
scalaNativePMain,
parent,
args,
redirectErrorOutput = false,
s => {
sbOut.append(s)
sbOut.append(Properties.lineSeparator)
},
s => {
sbErr.append(s)
sbErr.append(Properties.lineSeparator)
},
propagateError = true,
)
.map(_ => {
if (sbOut.isEmpty && sbErr.nonEmpty)
DecoderResponse.failed(
path.toURI,
s"$scalaNativeCLIDependency\n$scalaNativePMain\n$parent\n$args\n${sbErr.toString}",
)
else
DecoderResponse.success(path.toURI, sbOut.toString)
})
} catch {
case NonFatal(e) =>
scribe.error(e.toString())
Future.successful(DecoderResponse.failed(path.toURI, e))
}
}

private def decodeCFRFromClassFile(
path: AbsolutePath
): Future[DecoderResponse] = {
Expand Down
3 changes: 3 additions & 0 deletions metals/src/main/scala/scala/meta/internal/parsing/ClassFinder.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,16 +250,19 @@ sealed trait ClassFinderGranularity {

def isTasty: Boolean = this match {
case ClassFiles => false
case NIR => false
case Tasty => true
}

def extension: String = this match {
case ClassFiles => ".class"
case NIR => ".nir"
case Tasty => ".tasty"
}
}

object ClassFinderGranularity {
case object ClassFiles extends ClassFinderGranularity
case object NIR extends ClassFinderGranularity
case object Tasty extends ClassFinderGranularity
}
Loading