Skip to content

use new zinc 1.8 api for VirtualFile #18137

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

Merged
merged 10 commits into from
Jul 23, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Prev Previous commit
address comments
  • Loading branch information
bishabosha committed Jul 21, 2023
commit e3de91c04ef12262cc3f3390cbef5a3f76387c54
6 changes: 3 additions & 3 deletions compiler/src/dotty/tools/dotc/core/Contexts.scala
Original file line number Diff line number Diff line change
Expand Up @@ -172,10 +172,10 @@ object Contexts {
val local = incCallback
if local != null then op(local)

def incrementalEnabled: Boolean =
def runZincPhases: Boolean =
def forceRun = settings.YdumpSbtInc.value || settings.YforceSbtPhases.value
val local = incCallback
if local != null then local.enabled
else false
local != null && local.enabled || forceRun

/** The current plain printer */
def printerFn: Context => Printer = store(printerFnLoc)
Expand Down
3 changes: 1 addition & 2 deletions compiler/src/dotty/tools/dotc/sbt/ExtractAPI.scala
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,7 @@ class ExtractAPI extends Phase {
override def description: String = ExtractAPI.description

override def isRunnable(using Context): Boolean = {
def forceRun = ctx.settings.YdumpSbtInc.value || ctx.settings.YforceSbtPhases.value
super.isRunnable && (ctx.incrementalEnabled || forceRun)
super.isRunnable && ctx.runZincPhases
}

// Check no needed. Does not transform trees
Expand Down
14 changes: 3 additions & 11 deletions compiler/src/dotty/tools/dotc/sbt/ExtractDependencies.scala
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,7 @@ class ExtractDependencies extends Phase {
override def description: String = ExtractDependencies.description

override def isRunnable(using Context): Boolean = {
def forceRun = ctx.settings.YdumpSbtInc.value || ctx.settings.YforceSbtPhases.value
super.isRunnable && (ctx.incrementalEnabled || forceRun)
super.isRunnable && ctx.runZincPhases
}

// Check no needed. Does not transform trees
Expand Down Expand Up @@ -123,15 +122,8 @@ class ExtractDependencies extends Phase {
case Some(zip) if zip.jpath != null =>
binaryDependency(zip.jpath, binaryClassName)
case _ =>
case pf: PlainFile => // The dependency comes from a class file
// FIXME: pf.file is null for classfiles coming from the modulepath
// (handled by JrtClassPath) because they cannot be represented as
// java.io.File, since the `binaryDependency` callback must take a
// java.io.File, this means that we cannot record dependencies coming
// from the modulepath. For now this isn't a big deal since we only
// support having the standard Java library on the modulepath.
if pf.jpath != null then
binaryDependency(pf.jpath, binaryClassName)
case pf: PlainFile => // The dependency comes from a class file, Zinc handles JRT filesystem
binaryDependency(pf.jpath, binaryClassName)
case _ =>
internalError(s"Ignoring dependency $depFile of unknown class ${depFile.getClass}}", dep.from.srcPos)
}
Expand Down
15 changes: 0 additions & 15 deletions sbt-bridge/src/dotty/tools/xsbt/BasicPathBasedFile.java

This file was deleted.

53 changes: 43 additions & 10 deletions sbt-bridge/src/dotty/tools/xsbt/CompilerBridgeDriver.java
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@
import dotty.tools.dotc.core.Contexts;
import dotty.tools.dotc.util.SourceFile;
import dotty.tools.io.AbstractFile;
import dotty.tools.io.PlainFile;
import dotty.tools.io.Path;
import dotty.tools.io.Streamable;
import scala.collection.mutable.ListBuffer;
import scala.jdk.javaapi.CollectionConverters;
import scala.io.Codec;
Expand All @@ -20,6 +23,8 @@
import xsbti.compile.Output;

import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.util.Comparator;
import java.util.Collections;
import java.util.HashMap;
Expand Down Expand Up @@ -60,19 +65,20 @@ private static VirtualFile asVirtualFile(SourceFile sourceFile, DelegatingReport
return lookup.computeIfAbsent(sourceFile.file(), path -> {
reportMissingFile(reporter, sourceFile);
if (sourceFile.file().jpath() != null)
return new BasicPathBasedFile(sourceFile);
return new FallbackPathBasedFile(sourceFile);
else
return new PlaceholderVirtualFile(sourceFile);
return new FallbackVirtualFile(sourceFile);
});
}

private static void reportMissingFile(DelegatingReporter reporter, SourceFile sourceFile) {
String underline = String.join("", Collections.nCopies(sourceFile.path().length(), "^"));
String message =
sourceFile.path() + ": Missing virtual file\n" +
sourceFile.path() + ": Missing Zinc virtual file\n" +
underline + "\n" +
" Falling back to placeholder for the given source file (of class " + sourceFile.getClass().getName() + ")\n" +
" This is likely a bug in incremental compilation for the Scala 3 compiler. Please report it to the Scala 3 maintainers.";
" This is likely a bug in incremental compilation for the Scala 3 compiler.\n" +
" Please report it to the Scala 3 maintainers at https://github.com/lampepfl/dotty/issues.";
reporter.reportBasicWarning(message);
}

Expand All @@ -91,9 +97,19 @@ synchronized public void run(VirtualFile[] sources, AnalysisCallback callback, L
lookup.put(abstractFile, source);
}

DelegatingReporter reporter = new DelegatingReporter(delegate, (self, sourceFile) ->
asVirtualFile(sourceFile, self, lookup).id()
);
DelegatingReporter reporter = new DelegatingReporter(delegate, sourceFile -> {
// TODO: possible situation here where we use -from-tasty and TASTy source files but
// the reporter log is associated to a Scala source file?

// Zinc will use the output of this function to possibly lookup a mapped virtual file,
// e.g. convert `${ROOT}/Foo.scala` to `/path/to/Foo.scala` if it exists in the lookup map.
VirtualFile vf = lookup.get(sourceFile.file());
if (vf != null)
return vf.id();
else
// follow Zinc, which uses the path of the source file as a fallback.
return sourceFile.path();
});

IncrementalCallback incCallback = new IncrementalCallback(callback, sourceFile ->
asVirtualFile(sourceFile, reporter, lookup)
Expand Down Expand Up @@ -137,11 +153,28 @@ synchronized public void run(VirtualFile[] sources, AnalysisCallback callback, L
}

private static AbstractFile asDottyFile(VirtualFile virtualFile) {
if (virtualFile instanceof PathBasedFile)
return new ZincPlainFile((PathBasedFile) virtualFile);
if (virtualFile instanceof PathBasedFile) {
java.nio.file.Path path = ((PathBasedFile) virtualFile).toPath();
return new PlainFile(new Path(path));
}

try {
return new ZincVirtualFile(virtualFile);
return new dotty.tools.io.VirtualFile(virtualFile.name(), virtualFile.id()) {
{
// fill in the content
try (OutputStream output = output()) {
try (InputStream input = virtualFile.input()) {
Streamable.Bytes bytes = new Streamable.Bytes() {
@Override
public InputStream inputStream() {
return input;
}
};
output.write(bytes.toByteArray());
}
}
}
};
} catch (IOException e) {
throw new IllegalArgumentException("invalid file " + virtualFile.name(), e);
}
Expand Down
19 changes: 11 additions & 8 deletions sbt-bridge/src/dotty/tools/xsbt/DelegatingReporter.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,16 @@

final public class DelegatingReporter extends AbstractReporter {
private xsbti.Reporter delegate;
private final BiFunction<DelegatingReporter, SourceFile, String> baseLookup;
private final Function<SourceFile, String> lookup;

public DelegatingReporter(xsbti.Reporter delegate, BiFunction<DelegatingReporter, SourceFile, String> baseLookup) {
// A function that can lookup the `id` of the VirtualFile
// associated with a SourceFile. If there is not an associated virtual file,
// then it is the path of the SourceFile as a String.
private final Function<SourceFile, String> lookupVirtualFileId;

public DelegatingReporter(xsbti.Reporter delegate, Function<SourceFile, String> lookupVirtualFileId) {
super();
this.delegate = delegate;
this.baseLookup = baseLookup;
this.lookup = sourceFile -> baseLookup.apply(this, sourceFile);
this.lookupVirtualFileId = lookupVirtualFileId;
}

public void dropDelegate() {
Expand Down Expand Up @@ -60,15 +62,16 @@ public void doReport(Diagnostic dia, Context ctx) {
messageBuilder.append(System.lineSeparator()).append(explanation(message, ctx));
}

delegate.log(new Problem(position, messageBuilder.toString(), severity, rendered.toString(), diagnosticCode, actions, lookup));
delegate.log(new Problem(position, messageBuilder.toString(), severity, rendered.toString(), diagnosticCode, actions,
lookupVirtualFileId));
}

public void reportBasicWarning(String message) {
Position position = PositionBridge.noPosition;
Severity severity = Severity.Warn;
String diagnosticCode = "-1"; // no error code
List<CodeAction> actions = Collections.emptyList();
delegate.log(new Problem(position, message, severity, message, diagnosticCode, actions, lookup));
delegate.log(new Problem(position, message, severity, message, diagnosticCode, actions, lookupVirtualFileId));
}

private static Severity severityOf(int level) {
Expand All @@ -85,7 +88,7 @@ private static Severity severityOf(int level) {

private Position positionOf(SourcePosition pos) {
if (pos.exists()) {
return new PositionBridge(pos, lookup.apply(pos.source()));
return new PositionBridge(pos, lookupVirtualFileId.apply(pos.source()));
} else {
return PositionBridge.noPosition;
}
Expand Down
20 changes: 20 additions & 0 deletions sbt-bridge/src/dotty/tools/xsbt/FallbackPathBasedFile.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package dotty.tools.xsbt;

import dotty.tools.dotc.util.SourceFile;

/**A basic implementation of PathBasedFile that is only used when
* the real virtual file can not be found.
*
* See FallbackVirtualFile for more details.
*/
public class FallbackPathBasedFile extends FallbackVirtualFile implements xsbti.PathBasedFile {

public FallbackPathBasedFile(SourceFile sourceFile) {
super(sourceFile);
}

public java.nio.file.Path toPath() {
return sourceFile.file().jpath();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,18 @@
import java.io.InputStream;
import java.nio.charset.StandardCharsets;

public class PlaceholderVirtualFile extends xsbti.BasicVirtualFileRef implements xsbti.VirtualFile {
/**A basic implementation of VirtualFile that is only used when
* the real virtual file can not be found.
*
* This has a very basic implementation of contentHash that is almost certainly colliding more than the implementation
* in Zinc. It does not matter anyway as Zinc will recompile the associated source file, because it did not recieve the
* same virtual file back.
*/
public class FallbackVirtualFile extends xsbti.BasicVirtualFileRef implements xsbti.VirtualFile {

protected final SourceFile sourceFile;

public PlaceholderVirtualFile(SourceFile sourceFile) {
public FallbackVirtualFile(SourceFile sourceFile) {
super(sourceFile.path());
this.sourceFile = sourceFile;
}
Expand All @@ -23,7 +30,7 @@ public InputStream input() {

public long contentHash() {
int murmurHash3 = scala.util.hashing.MurmurHash3$.MODULE$.bytesHash(toBytes(sourceFile.content()));
return scala.util.hashing.package$.MODULE$.byteswap64((long) murmurHash3);
return (long) murmurHash3;
}

}
20 changes: 12 additions & 8 deletions sbt-bridge/src/dotty/tools/xsbt/Problem.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,18 +26,22 @@ final public class Problem implements xsbti.Problem {
private final Optional<String> _rendered;
private final String _diagnosticCode;
private final List<CodeAction> _actions;
private final Function<SourceFile, String> _lookup;

// A function that can lookup the `id` of the VirtualFile
// associated with a SourceFile. If there is not an associated virtual file,
// then it is the path of the SourceFile as a String.
private final Function<SourceFile, String> _lookupVirtualFileId;

public Problem(Position position, String message, Severity severity, String rendered, String diagnosticCode, List<CodeAction> actions,
Function<SourceFile, String> lookup) {
Function<SourceFile, String> lookupVirtualFileId) {
super();
this._position = position;
this._message = message;
this._severity = severity;
this._rendered = Optional.of(rendered);
this._diagnosticCode = diagnosticCode;
this._actions = actions;
this._lookup = lookup;
this._lookupVirtualFileId = lookupVirtualFileId;
}

public String category() {
Expand Down Expand Up @@ -86,23 +90,23 @@ public List<xsbti.Action> actions() {
// never getting called.
return _actions
.stream()
.map(action -> new Action(action.title(), OptionConverters.toJava(action.description()), toWorkspaceEdit(CollectionConverters.asJava(action.patches()), _lookup)))
.map(action -> new Action(action.title(), OptionConverters.toJava(action.description()), toWorkspaceEdit(CollectionConverters.asJava(action.patches()), _lookupVirtualFileId)))
.collect(toList());
}
}

private static WorkspaceEdit toWorkspaceEdit(List<ActionPatch> patches, Function<SourceFile, String> lookup) {
private static WorkspaceEdit toWorkspaceEdit(List<ActionPatch> patches, Function<SourceFile, String> lookupVirtualFileId) {
return new WorkspaceEdit(
patches
.stream()
.map(patch -> new TextEdit(positionOf(patch.srcPos(), lookup), patch.replacement()))
.map(patch -> new TextEdit(positionOf(patch.srcPos(), lookupVirtualFileId), patch.replacement()))
.collect(toList())
);
}

private static Position positionOf(SourcePosition pos, Function<SourceFile, String> lookup) {
private static Position positionOf(SourcePosition pos, Function<SourceFile, String> lookupVirtualFileId) {
if (pos.exists()){
return new PositionBridge(pos, lookup.apply(pos.source()));
return new PositionBridge(pos, lookupVirtualFileId.apply(pos.source()));
} else {
return PositionBridge.noPosition;
}
Expand Down
14 changes: 0 additions & 14 deletions sbt-bridge/src/dotty/tools/xsbt/ZincPlainFile.java

This file was deleted.

33 changes: 0 additions & 33 deletions sbt-bridge/src/dotty/tools/xsbt/ZincVirtualFile.java

This file was deleted.

2 changes: 1 addition & 1 deletion sbt-bridge/src/xsbt/CachedCompilerImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ synchronized public void run(File[] sources, DependencyChanges changes, Analysis

Context ctx = new ContextBase().initialCtx().fresh()
.setIncCallback(incCallback)
.setReporter(new DelegatingReporter(delegate, (self, source) -> source.file().absolutePath()));
.setReporter(new DelegatingReporter(delegate, source -> source.file().absolutePath()));

dotty.tools.dotc.reporting.Reporter reporter = Main.process(commandArguments(sources), ctx);
if (reporter.hasErrors()) {
Expand Down
2 changes: 1 addition & 1 deletion sbt-bridge/src/xsbt/DottydocRunner.java
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ public void run() {
args = retained.toArray(new String[retained.size()]);

Context ctx = new ContextBase().initialCtx().fresh()
.setReporter(new DelegatingReporter(delegate, (self, source) -> source.file().absolutePath()));
.setReporter(new DelegatingReporter(delegate, source -> source.file().absolutePath()));

try {
Class<?> dottydocMainClass = Class.forName("dotty.tools.dottydoc.Main");
Expand Down