diff --git a/de.peeeq.wurstscript/build.gradle b/de.peeeq.wurstscript/build.gradle index 9fd42ad1e..611bfc524 100644 --- a/de.peeeq.wurstscript/build.gradle +++ b/de.peeeq.wurstscript/build.gradle @@ -108,8 +108,8 @@ dependencies { implementation 'commons-lang:commons-lang:2.6' implementation 'com.github.albfernandez:juniversalchardet:2.4.0' implementation 'org.xerial:sqlite-jdbc:3.46.1.3' - implementation 'com.github.inwc3:jmpq3:e28f6999c0' - implementation 'com.github.inwc3:wc3libs:ac41f780a5e2dfc35310be4ed3267f23ab3fea44' + implementation 'com.github.inwc3:JMPQ3:v2.0.1' + implementation 'com.github.inwc3:wc3libs:5ad2e5be4c480bb14112222521e6bc5571d00076' implementation 'com.github.wurstscript:wurst-project-config:348fcd4ef5' implementation 'org.slf4j:slf4j-api:2.0.17' implementation 'ch.qos.logback:logback-classic:1.5.20' diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/Main.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/Main.java index 57752900d..23830f462 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/Main.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/Main.java @@ -137,7 +137,7 @@ public static void main(String[] args) { List mergedArgs = new ArrayList<>(asList(args)); if (workspaceroot != null) { WLogger.info("workspaceroot: " + workspaceroot); - List argsList = getCompileArgs(WFile.create(workspaceroot)); + List argsList = getCompileArgs(WFile.create(workspaceroot), runArgs.isBuild()); WLogger.info("workspaceroot: " + (argsList == null)); mergedArgs.addAll(argsList); compileArgs = new RunArgs(mergedArgs); diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/WurstCompilerJassImpl.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/WurstCompilerJassImpl.java index de2cd9c1d..62a73b79b 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/WurstCompilerJassImpl.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/WurstCompilerJassImpl.java @@ -33,7 +33,6 @@ import de.peeeq.wurstscript.types.TypesHelper; import de.peeeq.wurstscript.utils.LineOffsets; import de.peeeq.wurstscript.utils.NotNullList; -import de.peeeq.wurstscript.utils.TempDir; import de.peeeq.wurstscript.utils.Utils; import org.eclipse.jdt.annotation.Nullable; import org.eclipse.lsp4j.MessageType; @@ -41,6 +40,7 @@ import java.io.*; import java.lang.ref.WeakReference; +import java.nio.charset.StandardCharsets; import java.util.*; import java.util.Map.Entry; import java.util.function.Function; @@ -697,11 +697,8 @@ private CompilationUnit processMap(File file) { // extract mapscript: try { byte[] tempBytes = mapMpq.extractFile("war3map.j"); - File tempFile = File.createTempFile("war3map", ".j", TempDir.get()); // TODO work directly with bytes without temp file - tempFile.deleteOnExit(); - Files.write(tempBytes, tempFile); - if (isWurstGenerated(tempFile)) { + if (isWurstGenerated(tempBytes)) { // the war3map.j file was generated by wurst // this should not be the case, as we will get duplicate function errors in this case throw new AbortCompilationException( @@ -716,12 +713,8 @@ private CompilationUnit processMap(File file) { throw new AbortCompilationException("Could not create Wurst folder at " + wurstFolder + "."); } File wurstwar3map = new File(wurstFolder, "war3map.j"); - wurstwar3map.delete(); - if (tempFile.renameTo(wurstwar3map)) { - return parseFile(wurstwar3map); - } else { - throw new Error("Could not move war3map.j from " + tempFile + " to " + wurstwar3map); - } + java.nio.file.Files.write(wurstwar3map.toPath(), tempBytes); + return parseFile(wurstwar3map); } catch (RuntimeException e) { throw e; } catch (Exception e) { @@ -730,11 +723,12 @@ private CompilationUnit processMap(File file) { } - private boolean isWurstGenerated(File tempFile) { - try (FileReader fr = new FileReader(tempFile); BufferedReader in = new BufferedReader(fr)) { + private boolean isWurstGenerated(byte[] contents) { + try (BufferedReader in = new BufferedReader(new InputStreamReader( + new ByteArrayInputStream(contents), StandardCharsets.UTF_8))) { String firstLine = in.readLine(); WLogger.info("firstLine = '" + firstLine + "'"); - return firstLine.equals(JassPrinter.WURST_COMMENT); + return JassPrinter.WURST_COMMENT.equals(firstLine); } catch (IOException e) { WLogger.severe(e); } diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/languageserver/WurstCommands.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/languageserver/WurstCommands.java index d2f6f2ec9..978174737 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/languageserver/WurstCommands.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/languageserver/WurstCommands.java @@ -13,6 +13,7 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; +import java.util.ArrayList; import java.util.List; import java.util.Optional; import java.util.concurrent.CompletableFuture; @@ -90,7 +91,7 @@ private static CompletableFuture buildMap(WurstLanguageServer server, Ex } Optional map = mapPath.map(File::new); - List compileArgs = getCompileArgs(workspaceRoot); + List compileArgs = getCompileArgs(workspaceRoot, true); return server.worker().handle(new BuildMap(server, workspaceRoot, wc3Path, map, compileArgs)).thenApply(x -> x); } @@ -121,27 +122,54 @@ private static Optional getString(JsonObject options, String key) { private static final List defaultArgs = ImmutableList.of("-runcompiletimefunctions", "-injectobjects", "-stacktraces"); public static List getCompileArgs(WFile rootPath, String... additionalArgs) { + return getCompileArgs(rootPath, false, additionalArgs); + } + + /** + * Reads the workspace run-args file. A leading '-' is shared by run and build; + * a leading '+' is build-only and is normalized to '-' for the compiler. + */ + public static List getCompileArgs(WFile rootPath, boolean forBuild, String... additionalArgs) { try { Path configFile = Paths.get(rootPath.toString(), "wurst_run.args"); if (Files.exists(configFile)) { try (Stream lines = Files.lines(configFile)) { - List args = Stream.concat( - lines.filter(s -> s.startsWith("-")), - Stream.of(additionalArgs) - ).collect(Collectors.toList()); + List args = new ArrayList<>(lines + .map(String::trim) + .filter(s -> !s.isEmpty() && !s.startsWith("#")) + .filter(s -> s.startsWith("-") || (forBuild && s.startsWith("+"))) + .map(s -> forBuild && s.startsWith("+") ? "-" + s.substring(1) : s) + .collect(Collectors.toList())); + if (forBuild) { + addBuildDefault(args, "-opt"); + addBuildDefault(args, "-inline"); + addBuildDefault(args, "-localOptimizations"); + } + args.addAll(List.of(additionalArgs)); return WurstBuildConfig.fromWorkspaceRoot(rootPath).applyToCompileArgs(args); } } else { - - String cfg = String.join("\n", defaultArgs) + "\n"; + String cfg = String.join("\n", defaultArgs) + + "\n+opt\n+inline\n+localOptimizations\n"; Files.write(configFile, cfg.getBytes(Charsets.UTF_8)); - return WurstBuildConfig.fromWorkspaceRoot(rootPath).applyToCompileArgs( - Stream.concat(defaultArgs.stream(), Stream.of(additionalArgs)).collect(Collectors.toList()) - ); + List args = new ArrayList<>(defaultArgs); + if (forBuild) { + args.add("-opt"); + args.add("-inline"); + args.add("-localOptimizations"); + } + args.addAll(List.of(additionalArgs)); + return WurstBuildConfig.fromWorkspaceRoot(rootPath).applyToCompileArgs(args); } } catch (IOException e) { throw new RuntimeException("Could not access wurst_run.args config file", e); } } + private static void addBuildDefault(List args, String option) { + if (!args.contains(option)) { + args.add(option); + } + } + } diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/languageserver/requests/RunMap.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/languageserver/requests/RunMap.java index 8e9f7497d..8096129ea 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/languageserver/requests/RunMap.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/languageserver/requests/RunMap.java @@ -72,6 +72,7 @@ public Object execute(ModelManager modelManager) throws IOException { // TODO use normal compiler for this, avoid code duplication WurstGui gui = new WurstGuiImpl(getWorkspaceAbsolute()); try { + warnAboutRunOptimizations(gui); String ok = compileMap(modelManager, gui, projectConfig); if (ok != null) return ok; } catch (CompileError e) { @@ -91,6 +92,17 @@ public Object execute(ModelManager modelManager) throws IOException { return "ok"; // TODO } + private void warnAboutRunOptimizations(WurstGui gui) { + if (!runArgs.isOptimize() && !runArgs.isInline() && !runArgs.isLocalOptimizations()) { + return; + } + String message = "Run map is using compiler optimizations (opt/inline/localOptimizations), " + + "which can significantly slow the build. Put these options behind '+' in wurst_run.args " + + "or use Build Map to produce an optimized release map."; + WLogger.warning(message); + gui.sendProgress(message); + } + @Nullable private String compileMap(ModelManager modelManager, WurstGui gui, WurstProjectConfigData projectConfig) throws Exception { if (map.isPresent() && !map.get().exists()) { diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/map/importer/ImportFile.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/map/importer/ImportFile.java index dc915e4e9..8b6bb3def 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/map/importer/ImportFile.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/map/importer/ImportFile.java @@ -6,7 +6,6 @@ import de.peeeq.wurstio.mpq.MpqEditorFactory; import de.peeeq.wurstscript.RunArgs; import de.peeeq.wurstscript.WLogger; -import de.peeeq.wurstscript.utils.TempDir; import net.moonlightflower.wc3libs.bin.Wc3BinOutputStream; import net.moonlightflower.wc3libs.bin.app.IMP; @@ -297,9 +296,7 @@ public static void extractImportsFromMap(File mapFile, RunArgs runArgs) { try { File projectFolder = mapFile.getParentFile(); File importDirectory = getImportDirectory(projectFolder); - File tempMap = getCopyOfMap(mapFile); - - extractImportsFrom(importDirectory, tempMap, runArgs); + extractImportsFrom(importDirectory, mapFile, runArgs); } catch (Exception e) { WLogger.severe(e); JOptionPane.showMessageDialog(null, "Could not export objects (2): " + e.getMessage()); @@ -551,13 +548,6 @@ private static ImportResult insertImportedFiles_Cached(MpqEditor mpq, List return new ImportResult(filesProcessed, filesUpdated, filesDeleted, duration, !importsChanged); } - private static File getCopyOfMap(File mapFile) throws IOException { - File mapTemp = File.createTempFile("temp", "w3x", TempDir.get()); - mapTemp.deleteOnExit(); - Files.copy(mapFile, mapTemp); - return mapTemp; - } - private static File getImportDirectory(File projectFolder) { return new File(projectFolder, "imports"); } diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/mpq/Jmpq3BasedEditor.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/mpq/Jmpq3BasedEditor.java index 08e3e15f4..4afa64955 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/mpq/Jmpq3BasedEditor.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/mpq/Jmpq3BasedEditor.java @@ -1,20 +1,46 @@ package de.peeeq.wurstio.mpq; import com.google.common.base.Preconditions; -import systems.crigges.jmpq3.JMpqEditor; -import systems.crigges.jmpq3.JMpqException; -import systems.crigges.jmpq3.MPQOpenOption; +import org.inwc3.jmpq.MpqArchive; +import org.inwc3.jmpq.MpqArchiveWriter; +import org.inwc3.jmpq.MpqOpenOptions; +import org.inwc3.jmpq.MpqWriteOptions; import java.io.File; import java.io.FileNotFoundException; import java.io.IOException; -import java.lang.reflect.InvocationTargetException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.StandardCopyOption; +import java.nio.file.attribute.PosixFileAttributeView; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Locale; +import java.util.Map; +import java.util.Set; +import java.util.function.Consumer; class Jmpq3BasedEditor implements MpqEditor { - private final JMpqEditor editor; - - private JMpqEditor getEditor() { - return editor; + private final File mpqArchive; + private final boolean readonly; + private final MpqArchive archive; + private final List> changes = new ArrayList<>(); + private final Map stagedFiles = new HashMap<>(); + private final Set deletedFiles = new HashSet<>(); + private MpqArchiveWriter stagingWriter; + private boolean stagingWriterAvailable; + private boolean keepHeaderOffset = true; + private boolean archiveClosed; + private boolean closed; + + private MpqArchiveWriter getStagingWriter() throws IOException { + if (stagingWriter == null) { + stagingWriter = MpqArchiveWriter.from(archive, + MpqWriteOptions.defaults().withPrefix(keepHeaderOffset)); + } + return stagingWriter; } public Jmpq3BasedEditor(File mpqArchive, boolean readonly) throws Exception { @@ -22,78 +48,193 @@ public Jmpq3BasedEditor(File mpqArchive, boolean readonly) throws Exception { if (!mpqArchive.exists()) { throw new FileNotFoundException("not found: " + mpqArchive); } - this.editor = new JMpqEditor(mpqArchive, readonly ? MPQOpenOption.READ_ONLY : MPQOpenOption.FORCE_V0); - + Path resolvedArchive = mpqArchive.toPath().toRealPath(); + this.mpqArchive = resolvedArchive.toFile(); + this.readonly = readonly; + this.archive = MpqArchive.open(resolvedArchive, MpqOpenOptions.warcraft3()); + if (!readonly) { + try { + getStagingWriter(); + stagingWriterAvailable = true; + } catch (IOException e) { + stagingWriterAvailable = false; + } + } } static void createEmptyArchive(File mpqArchive) throws IOException { - try { - JMpqEditor.class.getMethod("createEmptyArchive", File.class).invoke(null, mpqArchive); - } catch (NoSuchMethodException e) { - throw new IOException("JMPQ3 is missing createEmptyArchive(File); update the JMPQ3 dependency.", e); - } catch (IllegalAccessException e) { - throw new IOException("Cannot access JMPQ3 createEmptyArchive(File).", e); - } catch (InvocationTargetException e) { - Throwable cause = e.getCause(); - if (cause instanceof IOException) { - throw (IOException) cause; - } - throw new IOException("JMPQ3 could not create an empty MPQ archive.", cause); - } + MpqArchiveWriter.create(MpqWriteOptions.defaults()).save(mpqArchive.toPath()); } @Override public void insertFile(String filenameInMpq, byte[] contents) { - getEditor().deleteFile(filenameInMpq); - getEditor().insertByteArray(filenameInMpq, contents); + ensureWritable(); + byte[] copy = contents.clone(); + stage(filenameInMpq, new StagedFile(copy, null), writer -> writer.put(filenameInMpq, copy)); } @Override public void insertFile(String filenameInMpq, File contents) throws Exception { - getEditor().deleteFile(filenameInMpq); - getEditor().insertFile(filenameInMpq, contents); + ensureWritable(); + Path path = contents.toPath(); + stage(filenameInMpq, new StagedFile(null, path), writer -> writer.put(filenameInMpq, path)); } @Override public boolean canWrite() { - return editor.isCanWrite(); + Path archivePath = mpqArchive.toPath().toAbsolutePath(); + Path parent = archivePath.getParent(); + return !readonly + && stagingWriterAvailable + && Files.isWritable(archivePath) + && parent != null + && Files.isWritable(parent); } @Override public byte[] extractFile(String fileToExtract) throws Exception { - return getEditor().extractFileAsBytes(fileToExtract); + String key = stagedKey(fileToExtract); + StagedFile staged = stagedFiles.get(key); + if (staged != null) { + return staged.read(); + } + if (deletedFiles.contains(key)) { + throw new FileNotFoundException("not found in staged MPQ: " + fileToExtract); + } + return archive.read(fileToExtract); } @Override public void deleteFile(String filenameInMpq) { - getEditor().deleteFile(filenameInMpq); + ensureWritable(); + String key = stagedKey(filenameInMpq); + stagedFiles.remove(key); + deletedFiles.add(key); + addChange(writer -> writer.remove(filenameInMpq)); } @Override public void close() throws IOException { - try { - editor.close(); - } catch (JMpqException e) { - throw new IOException(e); + if (closed) return; + if (readonly) { + closeArchive(); + closed = true; + return; + } + if (changes.isEmpty()) { + closeArchive(); + closed = true; + return; } + save(MpqWriteOptions.defaults()); } @Override public boolean hasFile(String fileName) { - return getEditor().hasFile(fileName); + String key = stagedKey(fileName); + if (deletedFiles.contains(key)) { + return false; + } + if (stagedFiles.containsKey(key)) { + return true; + } + return stagingWriter != null + ? stagingWriter.contains(fileName) + : archive.contains(fileName); } @Override public void setKeepHeaderOffset(boolean flag) { - editor.setKeepHeaderOffset(flag); + keepHeaderOffset = flag; } @Override public void closeWithCompression() throws IOException { + if (closed) return; + if (readonly) { + closeArchive(); + closed = true; + return; + } + if (changes.isEmpty()) { + closeArchive(); + closed = true; + return; + } + save(MpqWriteOptions.recompressed().withPrefix(keepHeaderOffset)); + } + + private void save(MpqWriteOptions options) throws IOException { + MpqArchiveWriter writer = MpqArchiveWriter.from(archive, options.withPrefix(keepHeaderOffset)); + for (Consumer change : changes) { + change.accept(writer); + } + Path temporaryArchive = null; + boolean installed = false; + try { + Path parent = mpqArchive.toPath().toAbsolutePath().getParent(); + temporaryArchive = Files.createTempFile(parent, ".wurst-mpq-", ".tmp"); + writer.save(temporaryArchive); + copyPosixPermissions(mpqArchive.toPath(), temporaryArchive); + closeArchive(); + Files.move(temporaryArchive, mpqArchive.toPath(), StandardCopyOption.REPLACE_EXISTING); + installed = true; + } finally { + closeArchive(); + closed = true; + if (!installed && temporaryArchive != null) { + Files.deleteIfExists(temporaryArchive); + } + } + } + + private static void copyPosixPermissions(Path source, Path target) throws IOException { + PosixFileAttributeView sourceView = Files.getFileAttributeView(source, PosixFileAttributeView.class); + PosixFileAttributeView targetView = Files.getFileAttributeView(target, PosixFileAttributeView.class); + if (sourceView != null && targetView != null) { + var attributes = sourceView.readAttributes(); + targetView.setPermissions(attributes.permissions()); + targetView.setGroup(attributes.group()); + } + } + + private void ensureWritable() { + if (!canWrite()) { + throw new IllegalStateException("MPQ archive is not writable: " + mpqArchive); + } + } + + private void closeArchive() { + if (!archiveClosed) { + archive.close(); + archiveClosed = true; + } + } + + private void stage(String filename, StagedFile staged, Consumer change) { + String key = stagedKey(filename); + stagedFiles.put(key, staged); + deletedFiles.remove(key); + addChange(change); + } + + private void addChange(Consumer change) { + changes.add(change); try { - editor.close(true, false, true); - } catch (JMpqException e) { - throw new IOException(e); + getStagingWriter(); + change.accept(stagingWriter); + } catch (IOException e) { + throw new IllegalStateException("Could not stage MPQ change", e); + } + } + + private static String stagedKey(String filename) { + return filename.replace('/', '\\').toLowerCase(Locale.ROOT); + } + + private record StagedFile(byte[] bytes, Path path) { + private byte[] read() throws IOException { + return path == null ? bytes.clone() : Files.readAllBytes(path); } } diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/LuaDispatchPreparation.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/LuaDispatchPreparation.java index 1deaf553b..0eb4dc0dd 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/LuaDispatchPreparation.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/LuaDispatchPreparation.java @@ -2,6 +2,7 @@ import de.peeeq.datastructures.UnionFind; import de.peeeq.wurstscript.ast.AstElementWithFuncName; +import de.peeeq.wurstscript.ast.AstElementWithTypeParameters; import de.peeeq.wurstscript.ast.ExprClosure; import de.peeeq.wurstscript.ast.FuncDef; import de.peeeq.wurstscript.jassIm.ImClass; @@ -10,6 +11,7 @@ import de.peeeq.wurstscript.jassIm.ImMethod; import de.peeeq.wurstscript.jassIm.ImProg; import de.peeeq.wurstscript.jassIm.ImType; +import de.peeeq.wurstscript.jassIm.ImTypeVarRef; import de.peeeq.wurstscript.jassIm.ImVars; import de.peeeq.wurstscript.translation.lua.translation.LuaIdentifiers; @@ -235,7 +237,7 @@ private static void addHierarchyAliases(ImMethod method, Set aliases, Ma if (semanticNames.isEmpty()) { return; } - String dispatchKey = dispatchSignatureKey(method); + String dispatchKey = dispatchParameterSignatureKey(method); collectHierarchyAliases(owner, method, dispatchKey, semanticNames, aliases, sortedMethodsByClass, new HashSet<>(), tr); } @@ -245,12 +247,15 @@ private static void collectHierarchyAliases(ImClass c, ImMethod method, String d return; } for (ImMethod candidate : sortedMethodsForClass(c, sortedMethodsByClass)) { - if (!dispatchKey.equals(dispatchSignatureKey(candidate))) { + if (!dispatchKey.equals(dispatchParameterSignatureKey(candidate))) { continue; } if (!sharesSemanticName(method, candidate, semanticNames, tr)) { continue; } + if (!compatibleReturnTypes(method, candidate, tr)) { + continue; + } String candidateName = candidate.getName(); if (!candidateName.isEmpty()) { aliases.add(candidateName); @@ -422,6 +427,128 @@ private static String dispatchSignatureKey(ImMethod method) { return sb.toString(); } + /** + * The runtime dispatch slot is selected by the receiver and parameters, not by the return + * type. This matters for interface methods returning {@code thistype}: the interface method + * resolves to the interface type while a module-provided implementation resolves to the + * concrete class type, but both still need the interface alias on the concrete class table. + */ + private static String dispatchParameterSignatureKey(ImMethod method) { + ImFunction implementation = resolveDispatchSignatureImplementation(method, new HashSet<>()); + if (implementation == null) { + return ""; + } + StringBuilder sb = new StringBuilder(); + ImVars params = implementation.getParameters(); + for (int i = 1; i < params.size(); i++) { + if (i > 1) { + sb.append(","); + } + sb.append(typeKey(params.get(i).getType())); + } + return sb.toString(); + } + + /** + * Alias a covariant implementation return, such as a concrete class returned for an + * interface method returning {@code thistype}, but keep unrelated same-name methods apart. + */ + public static boolean compatibleReturnTypes(ImMethod left, ImMethod right) { + return compatibleReturnTypes(left, right, null); + } + + public static boolean compatibleReturnTypes(ImMethod left, ImMethod right, ImTranslator tr) { + ImType leftReturnType = dispatchReturnType(left, tr); + ImType rightReturnType = dispatchReturnType(right, tr); + if (leftReturnType == null || rightReturnType == null) { + return false; + } + if (leftReturnType.equalsType(rightReturnType)) { + return true; + } + // A generic method's return type can still be represented by the owning type variable + // when comparing it with an erased/specialized override. The generic dispatch machinery + // already guarantees that relationship; the Lua alias check must not discard that slot. + if ((leftReturnType instanceof ImTypeVarRef || rightReturnType instanceof ImTypeVarRef) + && sameOverrideFamily(left, right)) { + return true; + } + // A specialized generic override can have concrete return types on both sides after + // elimination (Holder.get_it() -> Doubler.get_it() is one example). That is safe only + // when the IM method union links the two methods; unrelated same-name methods in a generic + // owner must still remain separate. + if ((hasGenericOwner(left) || hasGenericOwner(right) + || hasGenericSourceOwner(left) || hasGenericSourceOwner(right)) + && sameOverrideFamily(left, right)) { + return true; + } + if (!(leftReturnType instanceof ImClassType leftClassType) + || !(rightReturnType instanceof ImClassType rightClassType)) { + return false; + } + ImClass leftClass = leftClassType.getClassDef(); + ImClass rightClass = rightClassType.getClassDef(); + ImClass leftOwner = left == null ? null : left.attrClass(); + ImClass rightOwner = right == null ? null : right.attrClass(); + if (leftOwner != null && rightOwner != null && leftOwner != rightOwner) { + if (leftOwner.isSubclassOf(rightOwner)) { + return leftClass.isSubclassOf(rightClass); + } + if (rightOwner.isSubclassOf(leftOwner)) { + return rightClass.isSubclassOf(leftClass); + } + } + if (left != null && right != null && left.getIsAbstract() != right.getIsAbstract()) { + return left.getIsAbstract() + ? rightClass.isSubclassOf(leftClass) + : leftClass.isSubclassOf(rightClass); + } + return false; + } + + private static boolean sameOverrideFamily(ImMethod left, ImMethod right) { + return reaches(left, right, new HashSet<>()) || reaches(right, left, new HashSet<>()); + } + + private static boolean reaches(ImMethod current, ImMethod target, Set visited) { + if (current == null || !visited.add(current)) { + return false; + } + if (current == target) { + return true; + } + for (ImMethod subMethod : current.getSubMethods()) { + if (reaches(subMethod, target, visited)) { + return true; + } + } + return false; + } + + private static boolean hasGenericOwner(ImMethod method) { + return method != null + && method.attrClass() != null + && !method.attrClass().getTypeVariables().isEmpty(); + } + + private static boolean hasGenericSourceOwner(ImMethod method) { + return method != null + && method.attrTrace() instanceof FuncDef funcDef + && funcDef.attrNearestClassOrInterface() instanceof AstElementWithTypeParameters owner + && !owner.getTypeParameters().isEmpty(); + } + + private static ImType dispatchReturnType(ImMethod method, ImTranslator tr) { + ImFunction implementation = resolveDispatchSignatureImplementation(method, new HashSet<>()); + if (implementation != null) { + return implementation.getReturnType(); + } + if (tr != null && method != null && method.attrTrace() instanceof FuncDef funcDef) { + return funcDef.attrReturnTyp().imTranslateType(tr); + } + return null; + } + private static ImFunction resolveDispatchSignatureImplementation(ImMethod method, Set visited) { if (method == null || !visited.add(method)) { return null; diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/LuaTranslator.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/LuaTranslator.java index 8f168dcb8..b2c9247ae 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/LuaTranslator.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/LuaTranslator.java @@ -968,7 +968,8 @@ private void createMethods(ImClass c, LuaVariable classVar) { } ImMethod current = slotToImpl.get(slotName); if (current != null && directSlots.contains(slotName) - && implArity(chosen) != implArity(current)) { + && (implArity(chosen) != implArity(current) + || !LuaDispatchPreparation.compatibleReturnTypes(chosen, current))) { continue; } if (current == null || compareDispatchCandidates(c, chosen, current) < 0) { @@ -1036,11 +1037,13 @@ private Set collectDispatchSlotNames(ImClass receiverClass, List ambiguous = ambiguousSemanticNames(receiverClass); - Set classNames = new TreeSet<>(); - collectClassNamesInHierarchy(receiverClass, classNames, new HashSet<>()); - for (String className : classNames) { + List classes = collectClassesInHierarchy(receiverClass); + for (ImClass targetClass : classes) { + String className = targetClass.getName(); for (String semanticName : semanticNames) { - if (ambiguous.contains(semanticName)) { + if (ambiguous.contains(semanticName) + || (isInterfaceClass(targetClass) + && !hasCompatibleSemanticMethod(targetClass, groupMethods, semanticName))) { continue; } slotNames.add(dispatchSlotName(className + "_" + semanticName)); @@ -1095,16 +1098,48 @@ private Set ambiguousSemanticNames(ImClass c) { }); } - private void collectClassNamesInHierarchy(ImClass c, Set out, Set visited) { + private List collectClassesInHierarchy(ImClass c) { + List result = new ArrayList<>(); + collectClassesInHierarchy(c, result, new HashSet<>()); + result.sort(Comparator.comparing(this::classSortKey)); + return result; + } + + private void collectClassesInHierarchy(ImClass c, List out, Set visited) { if (c == null || !visited.add(c)) { return; } - out.add(c.getName()); + out.add(c); for (ImClassType sc : c.getSuperClasses()) { - collectClassNamesInHierarchy(sc.getClassDef(), out, visited); + collectClassesInHierarchy(sc.getClassDef(), out, visited); } } + private boolean hasCompatibleSemanticMethod(ImClass targetClass, List groupMethods, String semanticName) { + for (ImMethod candidate : collectMethodsInHierarchy(targetClass)) { + String candidateSemanticName = imTr.dispatchSegmentOf(candidate); + if (!semanticName.equals(candidateSemanticName) + && !semanticName.equals(sourceSemanticName(candidate))) { + continue; + } + for (ImMethod groupMethod : groupMethods) { + // The semantic alias is deliberately broader than the exact dispatch-group key: + // generic overrides may have different erased parameter types while still sharing + // the same runtime slot. Return compatibility is the part that must remain strict; + // otherwise an unrelated int value() and string value() can claim one another's + // class-qualified alias. + if (LuaDispatchPreparation.compatibleReturnTypes(groupMethod, candidate, imTr)) { + return true; + } + } + } + return false; + } + + private boolean isInterfaceClass(ImClass c) { + return c != null && c.attrTrace() instanceof InterfaceDef; + } + private List collectMethodsInHierarchy(ImClass c) { List result = new ArrayList<>(); collectMethodsInHierarchy(c, result, new HashSet<>()); diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/HotReloadPipelineTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/HotReloadPipelineTests.java index 1b4730648..b92189651 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/HotReloadPipelineTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/HotReloadPipelineTests.java @@ -11,9 +11,7 @@ import de.peeeq.wurstio.utils.FileUtils; import de.peeeq.wurstscript.gui.WurstGui; import de.peeeq.wurstscript.gui.WurstGuiLogger; -import org.testng.SkipException; import org.testng.annotations.Test; -import systems.crigges.jmpq3.JMpqEditor; import java.io.File; import java.nio.charset.StandardCharsets; @@ -136,10 +134,6 @@ public void cachedMapFileNameIsModeSpecific() throws Exception { @Test public void folderMapInputIsMaterializedAsCachedArchive() throws Exception { - if (!jmpqCreateEmptyArchiveAvailable()) { - throw new SkipException("Requires JMPQ3 createEmptyArchive(File)."); - } - File projectFolder = new File("./temp/testProject_folder_map_cache/"); File wurstFolder = new File(projectFolder, "wurst"); newCleanFolder(wurstFolder); @@ -170,15 +164,6 @@ public void folderMapInputIsMaterializedAsCachedArchive() throws Exception { } } - private boolean jmpqCreateEmptyArchiveAvailable() { - try { - JMpqEditor.class.getMethod("createEmptyArchive", File.class); - return true; - } catch (NoSuchMethodException e) { - return false; - } - } - @Test public void jhcrPipelineRenamesOutputScript() throws Exception { File projectFolder = new File("./temp/testProject_jhcr_output/"); diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/InterfaceTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/InterfaceTests.java index 474a625f6..84bb5e3f7 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/InterfaceTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/InterfaceTests.java @@ -31,6 +31,42 @@ public void simple() { ); } + @Test + public void interfaceDispatchThroughNestedModuleWorksInBothBackends() { + test().testLua(true).luaOnly(false).executeProg().lines( + "package test", + "native testSuccess()", + "interface Greeter", + " function greet() returns int", + "module FirstGreeter", + " function greet() returns int", + " return 1", + "module NestedFirstGreeter", + " use FirstGreeter", + "module SecondGreeter", + " function greet() returns int", + " return 2", + "module GreeterCaller", + " function call(Greeter greeter) returns int", + " return greeter.greet()", + "class First implements Greeter", + " use NestedFirstGreeter", + " use GreeterCaller", + "class Second implements Greeter", + " use SecondGreeter", + " use GreeterCaller", + "init", + " Greeter first = new First()", + " Greeter second = new Second()", + " First firstObject = new First()", + " Second secondObject = new Second()", + " if first.greet() == 1 and second.greet() == 2", + " if firstObject.call(second) == 2 and secondObject.call(first) == 1", + " testSuccess()", + "endpackage" + ); + } + @Test public void swap() { testAssertOkLines(true, diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaTranslationTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaTranslationTests.java index 0b7ccddae..cdb7764a7 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaTranslationTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaTranslationTests.java @@ -603,6 +603,145 @@ public void moduleProvidedOverloadedOverrideDoesNotCollapseLuaSlots() { assertContainsRegex(compiled, "Child\\.Base(?:_M)?_setup" + Pattern.quote(overriddenSlots.get(0)) + "\\s*=\\s*Child_Child_setup"); } + @Test + public void moduleProvidedInterfaceDispatchSurvivesLuaOptimizations() { + String compiled = compileLuaWithRunArgs( + "LuaTranslationTests_moduleProvidedInterfaceDispatchSurvivesLuaOptimizations", + false, + "package Test", + "interface Greeter", + " function greet() returns thistype", + "module GreeterLifecycle", + " abstract function greet() returns thistype", + "module FirstGreeter", + " use GreeterLifecycle", + " override function greet() returns thistype", + " return this", + "module NestedFirstGreeter", + " use FirstGreeter", + "module SecondGreeter", + " use GreeterLifecycle", + " override function greet() returns thistype", + " return this", + "module GreeterCaller", + " function call(Greeter greeter) returns Greeter", + " return greeter.greet()", + "class First implements Greeter", + " use NestedFirstGreeter", + " use GreeterCaller", + "class Second implements Greeter", + " use SecondGreeter", + " use GreeterCaller", + "init", + " Greeter first = new First()", + " Greeter second = new Second()", + " First firstObject = new First()", + " Second secondObject = new Second()", + " Greeter firstResult = firstObject.call(second)", + " Greeter secondResult = secondObject.call(first)" + ); + Matcher callMatcher = Pattern.compile("return greeter\\d*:(\\w+)\\(").matcher(compiled); + List slots = new ArrayList<>(); + while (callMatcher.find() && !slots.contains(callMatcher.group(1))) { + slots.add(callMatcher.group(1)); + } + assertEquals("Both optimized module callers must use one interface dispatch slot.", 1, slots.size()); + String slot = slots.get(0); + assertContainsRegex(compiled, "First\\.[^\\n]*" + Pattern.quote(slot) + "\\s*=\\s*First_[^\\n]*greet"); + assertContainsRegex(compiled, "Second\\.[^\\n]*" + Pattern.quote(slot) + "\\s*=\\s*Second_[^\\n]*greet"); + } + + @Test + public void incompatibleSameNameInterfaceReturnsDoNotAliasInLua() { + String compiled = compileLuaWithRunArgs( + "LuaTranslationTests_incompatibleSameNameInterfaceReturnsDoNotAliasInLua", + false, + "package Test", + "interface IntValue", + " function value() returns int", + "interface StringValue", + " function value() returns string", + " return \"default\"", + "module IntValueImpl", + " function value() returns int", + " return 1", + "class Both implements IntValue, StringValue", + " use IntValueImpl", + "@noinline function readInt(IntValue value) returns int", + " return value.value()", + "@noinline function readString(StringValue value) returns string", + " return value.value()", + "init", + " readInt(new Both())", + " readString(new Both())" + ); + + assertContainsRegex(compiled, "return value:Both_IntValueImpl_value\\("); + assertContainsRegex(compiled, "Both\\.StringValue_value\\s*=\\s*StringValue_StringValue_value"); + assertDoesNotContainRegex(compiled, "Both\\.IntValue_value\\s*=\\s*StringValue_StringValue_value"); + } + + @Test + public void genericOwnersDoNotMakeIncompatibleInterfaceReturnsAliasInLua() { + String compiled = compileLuaWithRunArgs( + "LuaTranslationTests_genericOwnersDoNotMakeIncompatibleInterfaceReturnsAliasInLua", + false, + "package Test", + "interface IntValue", + " function value() returns int", + "interface StringValue", + " function value() returns string", + " return \"default\"", + "module IntValueImpl", + " function value() returns int", + " return 1", + "class Both implements IntValue, StringValue", + " use IntValueImpl", + "@noinline function readInt(IntValue value) returns int", + " return value.value()", + "@noinline function readString(StringValue value) returns string", + " return value.value()", + "init", + " readInt(new Both())", + " readString(new Both())" + ); + + assertContainsRegex(compiled, "Both\\.StringValue_value\\s*=\\s*StringValue_StringValue_value"); + assertDoesNotContainRegex(compiled, "Both\\.StringValue_value\\s*=\\s*Both_[^\\n]*IntValueImpl_value"); + } + + @Test + public void superclassReturnDoesNotReplaceCovariantInterfaceSlotInLua() { + String compiled = compileLuaWithRunArgs( + "LuaTranslationTests_superclassReturnDoesNotReplaceCovariantInterfaceSlotInLua", + false, + "package Test", + "class Base", + "class Derived extends Base", + "interface DerivedValue", + " function value() returns Derived", + " return new Derived()", + "interface BaseValue", + " function value() returns Base", + "module BaseValueImpl", + " function value() returns Base", + " return new Base()", + "class Both implements DerivedValue, BaseValue", + " use BaseValueImpl", + "@noinline function readDerived(DerivedValue value) returns Derived", + " return value.value()", + "@noinline function readBase(BaseValue value) returns Base", + " return value.value()", + "init", + " readDerived(new Both())", + " readBase(new Both())" + ); + + assertContainsRegex(compiled, "Both\\.DerivedValue_DerivedValue_value\\s*=\\s*DerivedValue_DerivedValue_value"); + assertContainsRegex(compiled, "Both\\.BaseValue_value\\s*=\\s*Both_Both_BaseValueImpl_value"); + assertDoesNotContainRegex(compiled, "Both\\.DerivedValue_value\\s*=\\s*BaseValue_BaseValue_value"); + } + @Test public void multiLevelOverloadedOverridesKeepDistinctLuaSlots() { String compiled = compileLuaWithRunArgs( @@ -1394,8 +1533,9 @@ public void legacyGenericToIndexFieldAssignmentRoundTripsInLua() { @Test public void genericOverrideChainBindsRootSlotToMostSpecificImplInLua() throws IOException { + String outputFile = "test-output/lua/LuaTranslationTests_genericOverrideChainBindsRootSlotToMostSpecificImplInLua.lua"; test().testLua(true).compilationUnits(genericOverrideReproUnits()); - String compiled = Files.toString(new File("test-output/lua/LuaTranslationTests_genericOverrideChainBindsRootSlotToMostSpecificImplInLua.lua"), Charsets.UTF_8); + String compiled = Files.toString(new File(outputFile), Charsets.UTF_8); Matcher slotMatcher = Pattern.compile("FSM_currentState:([A-Za-z0-9_]*_update)\\(").matcher(compiled); assertTrue("Expected FSM to dispatch through a virtual *_update slot.", slotMatcher.find()); @@ -1406,6 +1546,11 @@ public void genericOverrideChainBindsRootSlotToMostSpecificImplInLua() throws IO assertContainsRegex(compiled, state + "\\." + dispatchedSlot + "\\s*=\\s*" + state + "_" + state + "_update"); assertDoesNotContainRegex(compiled, state + "\\." + dispatchedSlot + "\\s*=\\s*NoOpState_NoOpState_update"); } + + GlobalCaches.clearAll(); + test().testLua(true).compilationUnits(genericOverrideReproUnits()); + String compiledAgain = Files.toString(new File(outputFile), Charsets.UTF_8); + assertEquals("The root-slot fixture must emit deterministic Lua across compilations.", compiled, compiledAgain); } @Test diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/MpqTest.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/MpqTest.java index cb2f51e72..67c4302bf 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/MpqTest.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/MpqTest.java @@ -10,6 +10,10 @@ import java.io.File; import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Path; +import java.nio.file.attribute.PosixFilePermission; +import java.util.Set; import java.util.Optional; public class MpqTest { @@ -74,4 +78,38 @@ public void test_delete() throws Exception { } } + @Test + public void test_stagedChangesAreVisibleBeforeClose() throws Exception { + byte[] contents = "staged contents".getBytes(StandardCharsets.UTF_8); + try (MpqEditor edit = MpqEditorFactory.getEditor(Optional.of(new File(TEST_W3X)))) { + edit.insertFile("staged.txt", contents); + Assert.assertTrue(edit.hasFile("staged.txt")); + Assert.assertEquals(edit.extractFile("STAGED.TXT"), contents); + + edit.deleteFile("staged.txt"); + Assert.assertFalse(edit.hasFile("staged.txt")); + } + } + + @Test + public void test_readOnlyMembershipDoesNotRequireWriter() throws Exception { + try (MpqEditor edit = MpqEditorFactory.getEditor(Optional.of(new File(TEST_W3X)), true)) { + Assert.assertTrue(edit.hasFile("war3map.j")); + Assert.assertFalse(edit.hasFile("missing.txt")); + } + } + + @Test + public void test_preservesPosixPermissions() throws Exception { + Path map = new File(TEST_W3X).toPath(); + if (!java.nio.file.Files.getFileStore(map).supportsFileAttributeView("posix")) { + return; + } + Set permissions = java.nio.file.Files.getPosixFilePermissions(map); + try (MpqEditor edit = MpqEditorFactory.getEditor(Optional.of(new File(TEST_W3X)))) { + edit.insertFile("permissions.txt", "permissions".getBytes(StandardCharsets.UTF_8)); + } + Assert.assertEquals(java.nio.file.Files.getPosixFilePermissions(map), permissions); + } + } diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/WurstCommandsTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/WurstCommandsTests.java new file mode 100644 index 000000000..cbc51d451 --- /dev/null +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/WurstCommandsTests.java @@ -0,0 +1,54 @@ +package tests.wurstscript.tests; + +import de.peeeq.wurstio.languageserver.WFile; +import de.peeeq.wurstio.languageserver.WurstCommands; +import org.testng.annotations.Test; + +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.List; + +import static org.testng.Assert.assertFalse; +import static org.testng.Assert.assertTrue; + +public class WurstCommandsTests { + + @Test + public void buildOnlyRunArgsAreExcludedFromRunAndEnabledForBuild() throws Exception { + Path project = Files.createTempDirectory("wurst-run-args-policy"); + Files.writeString(project.resolve("wurst.build"), "projectName: PolicyTest\n"); + Files.writeString(project.resolve("wurst_run.args"), """ + -stacktraces + +inline + +localOptimizations + # comments and blank lines are ignored + + """); + + WFile root = WFile.create(project.toFile()); + List runArgs = WurstCommands.getCompileArgs(root); + List buildArgs = WurstCommands.getCompileArgs(root, true); + + assertTrue(runArgs.contains("-stacktraces")); + assertFalse(runArgs.contains("-inline")); + assertFalse(runArgs.contains("-localOptimizations")); + assertTrue(buildArgs.contains("-stacktraces")); + assertTrue(buildArgs.contains("-inline")); + assertTrue(buildArgs.contains("-localOptimizations")); + assertTrue(buildArgs.contains("-opt"), "builds enable output optimization by default"); + } + + @Test + public void missingRunArgsFileDocumentsBuildOnlyDefaults() throws Exception { + Path project = Files.createTempDirectory("wurst-run-args-defaults"); + Files.writeString(project.resolve("wurst.build"), "projectName: DefaultsTest\n"); + + WFile root = WFile.create(project.toFile()); + WurstCommands.getCompileArgs(root); + + String config = Files.readString(project.resolve("wurst_run.args")); + assertTrue(config.contains("+opt")); + assertTrue(config.contains("+inline")); + assertTrue(config.contains("+localOptimizations")); + } +}