-
Notifications
You must be signed in to change notification settings - Fork 323
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
Exporting non-existing symbols fails with compiler error #7960
Changes from all commits
d32e836
4833480
93fba46
170309c
e0faeb3
6ac76b0
dec8dac
30d26f1
953058e
a6c47ed
1678ae2
8cd1422
4ca0526
e641230
f63c7a4
0b8b887
4306914
94e08bd
2872ede
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,146 @@ | ||
package org.enso.compiler.pass.analyse; | ||
|
||
import java.util.ArrayList; | ||
import java.util.List; | ||
import java.util.UUID; | ||
import org.enso.compiler.context.InlineContext; | ||
import org.enso.compiler.context.ModuleContext; | ||
import org.enso.compiler.core.IR; | ||
import org.enso.compiler.core.ir.Expression; | ||
import org.enso.compiler.core.ir.Module; | ||
import org.enso.compiler.core.ir.expression.errors.ImportExport; | ||
import org.enso.compiler.core.ir.module.scope.Export; | ||
import org.enso.compiler.data.BindingsMap; | ||
import org.enso.compiler.pass.IRPass; | ||
import org.enso.interpreter.util.ScalaConversions; | ||
import scala.collection.immutable.Seq; | ||
import scala.jdk.javaapi.CollectionConverters; | ||
|
||
/** | ||
* This pass ensures that all the symbols that are exported exist. If not, an IR error is generated. | ||
*/ | ||
public final class ExportSymbolAnalysis implements IRPass { | ||
public static final ExportSymbolAnalysis INSTANCE = new ExportSymbolAnalysis(); | ||
private static scala.collection.immutable.List<IRPass> precursorPasses; | ||
private UUID uuid; | ||
|
||
private ExportSymbolAnalysis() {} | ||
|
||
@Override | ||
public UUID key() { | ||
return null; | ||
} | ||
|
||
@Override | ||
public void org$enso$compiler$pass$IRPass$_setter_$key_$eq(UUID v) { | ||
this.uuid = v; | ||
} | ||
|
||
@Override | ||
public Seq<IRPass> precursorPasses() { | ||
if (precursorPasses == null) { | ||
List<IRPass> passes = List.of( | ||
BindingAnalysis$.MODULE$, | ||
ImportSymbolAnalysis$.MODULE$ | ||
); | ||
precursorPasses = CollectionConverters.asScala(passes).toList(); | ||
} | ||
return precursorPasses; | ||
} | ||
|
||
@Override | ||
public Seq<IRPass> invalidatedPasses() { | ||
return ScalaConversions.nil(); | ||
} | ||
|
||
@Override | ||
public Module runModule(Module moduleIr, ModuleContext moduleContext) { | ||
List<Export> exportErrors = new ArrayList<>(); | ||
var bindingsMap = (BindingsMap) moduleIr.passData().get(BindingAnalysis$.MODULE$).get(); | ||
|
||
moduleIr.exports().foreach(export -> switch (export) { | ||
case Export.Module exportMod -> { | ||
var exportNameParts = exportMod.name().parts(); | ||
var symbolName = exportMod.name().parts().last(); | ||
assert exportNameParts.size() > 1; | ||
var moduleOrTypeName = exportNameParts.apply(exportNameParts.size() - 2); | ||
var foundResolvedExp = findResolvedExportForIr(export, bindingsMap); | ||
if (foundResolvedExp == null) { | ||
exportErrors.add( | ||
ImportExport.apply( | ||
symbolName, | ||
new ImportExport.SymbolDoesNotExist(symbolName.name(), moduleOrTypeName.name()), | ||
ImportExport.apply$default$3(), | ||
ImportExport.apply$default$4() | ||
) | ||
); | ||
} else { | ||
if (exportMod.onlyNames().isDefined()) { | ||
assert exportMod.onlyNames().isDefined(); | ||
var exportedSymbols = exportMod.onlyNames().get(); | ||
exportedSymbols.foreach(exportedSymbol -> { | ||
var foundSymbols = foundResolvedExp.target().findExportedSymbolsFor(exportedSymbol.name()); | ||
if (foundSymbols.isEmpty()) { | ||
exportErrors.add( | ||
ImportExport.apply( | ||
exportedSymbol, | ||
new ImportExport.SymbolDoesNotExist(exportedSymbol.name(), moduleOrTypeName.name()), | ||
ImportExport.apply$default$3(), | ||
ImportExport.apply$default$4() | ||
) | ||
Comment on lines
+85
to
+90
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It makes me sad to see this done in not-Scala, but I know this is the general direction. But it still makes me sad 🙁 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Apart from some weird perks, like calling methods with default arguments, or methods on objects, there is nothing stopping us from calling Scala stuff from Java stuff anymore. It does not look nice, but it is the most viable compromise for now. |
||
); | ||
} | ||
return null; | ||
}); | ||
} | ||
} | ||
yield null; | ||
} | ||
default -> export; | ||
}); | ||
|
||
if (exportErrors.isEmpty()) { | ||
return moduleIr; | ||
} else { | ||
return moduleIr.copy( | ||
moduleIr.imports(), | ||
CollectionConverters.asScala(exportErrors).toList(), | ||
moduleIr.bindings(), | ||
moduleIr.location(), | ||
moduleIr.passData(), | ||
moduleIr.diagnostics(), | ||
moduleIr.id() | ||
); | ||
} | ||
} | ||
|
||
@Override | ||
public Expression runExpression(Expression ir, InlineContext inlineContext) { | ||
return ir; | ||
} | ||
|
||
/** | ||
* Finds a resolved export that corresponds to the export IR. | ||
* @param exportIr Export IR that is being resolved | ||
* @param bindingsMap Bindings map of the module that contains the export IR | ||
* @return null if no resolved export was found, otherwise the resolved export | ||
*/ | ||
private BindingsMap.ExportedModule findResolvedExportForIr(Export exportIr, BindingsMap bindingsMap) { | ||
switch (exportIr) { | ||
case Export.Module exportedModIr -> { | ||
var exportedModName = exportedModIr.name().name(); | ||
var foundResolvedExp = bindingsMap.resolvedExports().find(resolvedExport -> { | ||
var resolvedExportName = resolvedExport.target().qualifiedName(); | ||
return resolvedExportName.toString().equals(exportedModName); | ||
}); | ||
return foundResolvedExp.isEmpty() ? null : foundResolvedExp.get(); | ||
} | ||
default -> throw new IllegalStateException("Unexpected value: " + exportIr); | ||
} | ||
} | ||
|
||
@Override | ||
public <T extends IR> T updateMetadataInDuplicate(T sourceIr, T copyOfIr) { | ||
return IRPass.super.updateMetadataInDuplicate(sourceIr, copyOfIr); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -30,15 +30,11 @@ | |
* Inserts errors into imports/exports IRs if the above conditions are violated. | ||
*/ | ||
public final class PrivateModuleAnalysis implements IRPass { | ||
private static final PrivateModuleAnalysis singleton = new PrivateModuleAnalysis(); | ||
public static final PrivateModuleAnalysis INSTANCE = new PrivateModuleAnalysis(); | ||
private UUID uuid; | ||
|
||
private PrivateModuleAnalysis() {} | ||
|
||
public static PrivateModuleAnalysis getInstance() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Isn't getter better than a field? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In most scenarios, I would say so. But here, I wanted to get as closest to |
||
return singleton; | ||
} | ||
|
||
@Override | ||
public void org$enso$compiler$pass$IRPass$_setter_$key_$eq(UUID v) { | ||
this.uuid = v; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file does not exist, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep.
It's my fault, I forgot to remove this export after removing the file, sorry.
Thanks for adding a pass that catches such errors! ❤️
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, exactly. This is why it is now removed - the compilation now fails with this pass.