Skip to content

Commit

Permalink
address comments
Browse files Browse the repository at this point in the history
  • Loading branch information
bishabosha committed Jul 21, 2023
1 parent 246dfc4 commit e3de91c
Show file tree
Hide file tree
Showing 13 changed files with 105 additions and 109 deletions.
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

0 comments on commit e3de91c

Please sign in to comment.