-
Notifications
You must be signed in to change notification settings - Fork 29
Preserve externally visible names during optimization #1277
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
Changes from all commits
834228d
4e1b5e8
7470b45
5ed6101
d32a923
d0b45cf
fd7d4b4
67aaea3
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 |
|---|---|---|
|
|
@@ -5,18 +5,32 @@ | |
| import de.peeeq.wurstscript.jassIm.ImVar; | ||
| import de.peeeq.wurstscript.translation.imtranslation.ImHelper; | ||
| import de.peeeq.wurstscript.translation.imtranslation.ImTranslator; | ||
| import de.peeeq.wurstscript.validation.TRVEHelper; | ||
| import de.peeeq.wurstscript.validation.NamePreservation; | ||
|
|
||
| import java.util.HashSet; | ||
| import java.util.Set; | ||
|
|
||
| public class ImCompressor { | ||
|
|
||
| private final ImTranslator trans; | ||
| private final ImProg prog; | ||
| private final NameGenerator ng; | ||
| private final Set<String> preservedNames = new HashSet<>(); | ||
|
|
||
| public ImCompressor(ImTranslator translator) { | ||
| this.trans = translator; | ||
| this.prog = translator.getImProg(); | ||
| ng = new NameGenerator(); | ||
| for (ImVar global : prog.getGlobals()) { | ||
| if (NamePreservation.isPreserved(global)) { | ||
| preservedNames.add(global.getName()); | ||
| } | ||
| } | ||
| for (ImFunction function : ImHelper.calculateFunctionsOfProg(prog)) { | ||
| if (NamePreservation.isPreserved(function)) { | ||
| preservedNames.add(function.getName()); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| public void compressNames() { | ||
|
|
@@ -27,21 +41,21 @@ public void compressNames() { | |
|
|
||
| public void compressGlobals() { | ||
| for (final ImVar global : prog.getGlobals()) { | ||
| if (global.getIsBJ() || TRVEHelper.protectedVariables.contains(global.getName())) { | ||
| // do not rename bj constants | ||
| // do not rename TRVE vars | ||
| if (global.getIsBJ() || NamePreservation.isPreserved(global)) { | ||
| // do not rename bj constants or names exposed to Warcraft III | ||
| continue; | ||
| } | ||
|
|
||
| String replacement = ng.getUniqueToken(); | ||
| String replacement = nextCompressedName(); | ||
|
|
||
| global.setName(replacement); | ||
| } | ||
| } | ||
|
|
||
| public void compressFunctions() { | ||
| for (ImFunction func : ImHelper.calculateFunctionsOfProg(prog)) { | ||
| if (func.isNative() || func.isBj() || func.isCompiletime() || func.isExtern()) { | ||
| if (func.isNative() || func.isBj() || func.isCompiletime() || func.isExtern() | ||
| || NamePreservation.isPreserved(func)) { | ||
| // do not rename builtin an bj functions | ||
| continue; | ||
|
Comment on lines
+57
to
60
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.
When a preserved identifier matches a token produced by Useful? React with 👍 / 👎. |
||
| } | ||
|
|
@@ -50,12 +64,20 @@ public void compressFunctions() { | |
| // do not rename main and config functions | ||
| continue; | ||
| } | ||
| String rname = ng.getUniqueToken(); | ||
| String rname = nextCompressedName(); | ||
| func.setName(rname); | ||
| } | ||
|
|
||
| } | ||
|
|
||
| private String nextCompressedName() { | ||
| String replacement; | ||
| do { | ||
| replacement = ng.getUniqueToken(); | ||
| } while (preservedNames.contains(replacement)); | ||
| return replacement; | ||
| } | ||
|
|
||
| private void compressLocals(ImFunction func) { | ||
| // TODO compressing locals should not use the global name pool but use a own pool | ||
| for (ImVar local : func.getParameters()) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,7 +19,7 @@ | |
| import de.peeeq.wurstscript.types.*; | ||
| import de.peeeq.wurstscript.utils.Pair; | ||
| import de.peeeq.wurstscript.utils.Utils; | ||
| import de.peeeq.wurstscript.validation.TRVEHelper; | ||
| import de.peeeq.wurstscript.validation.NamePreservation; | ||
| import de.peeeq.wurstscript.validation.WurstValidator; | ||
| import it.unimi.dsi.fastutil.objects.Object2ObjectLinkedOpenHashMap; | ||
| import it.unimi.dsi.fastutil.objects.ObjectLinkedOpenHashSet; | ||
|
|
@@ -1067,6 +1067,9 @@ public ImFunction getFuncFor(TranslatedToImFunction funcDef) { | |
| if (m instanceof Annotation) { | ||
| Annotation annotation = (Annotation) m; | ||
| flags.add(new FunctionFlagAnnotation(annotation.getAnnotationType())); | ||
| if (NamePreservation.isPreserveAnnotation(annotation.getAnnotationType())) { | ||
| flags.add(PRESERVE_NAME); | ||
|
Comment on lines
+1070
to
+1071
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.
When two legal top-level overloads with the same source name both carry AGENTS.md reference: AGENTS.md:L229-L235 Useful? React with 👍 / 👎. |
||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
@@ -1462,13 +1465,18 @@ private void calculateCallRelationsAndVariables(boolean includeUsedVariables) { | |
| final ImFunction conf = getConfFunc(); | ||
| if (conf != null && conf != main) calculateCallRelations(conf, includeUsedVariables); | ||
|
|
||
| // mark protected globals as read | ||
| // TRVEHelper.protectedVariables is presumably a HashSet<String> (O(1) contains) | ||
| for (ImVar global : imProg.getGlobals()) { | ||
| if (TRVEHelper.protectedVariables.contains(global.getName())) { | ||
| readVariables.add(global); | ||
| // Preserved functions are externally visible entry points even when no Wurst code calls | ||
| // them. Keep their bodies and everything they call reachable for both backends. | ||
| for (ImFunction function : ImHelper.calculateFunctionsOfProg(imProg)) { | ||
| if (NamePreservation.isPreserved(function)) { | ||
| calculateCallRelations(function, includeUsedVariables); | ||
| } | ||
| } | ||
|
|
||
| // Mark externally visible globals as read so they survive garbage collection. | ||
| for (ImVar global : imProg.getGlobals()) { | ||
| if (NamePreservation.isPreserved(global)) readVariables.add(global); | ||
| } | ||
| } | ||
|
|
||
| private void calculateCallRelations(ImFunction rootFunction, boolean includeUsedVariables) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,6 +14,7 @@ | |
| import de.peeeq.wurstscript.types.TypesHelper; | ||
| import de.peeeq.wurstscript.utils.Lazy; | ||
| import de.peeeq.wurstscript.utils.Utils; | ||
| import de.peeeq.wurstscript.validation.NamePreservation; | ||
|
|
||
| import java.util.*; | ||
| import java.util.stream.Collectors; | ||
|
|
@@ -156,8 +157,10 @@ private static final class LazyArrayDefault { | |
| @Override | ||
| public LuaVariable initFor(ImVar a) { | ||
| String name = a.getName(); | ||
| if (!a.getIsBJ()) { | ||
| if (!a.getIsBJ() && !NamePreservation.isPreserved(a)) { | ||
| name = uniqueName(name); | ||
| } else { | ||
| usedNames.add(name); | ||
| } | ||
| return LuaAst.LuaVariable(name, LuaAst.LuaNoExpr()); | ||
| } | ||
|
|
@@ -168,9 +171,10 @@ public LuaVariable initFor(ImVar a) { | |
| @Override | ||
| public LuaFunction initFor(ImFunction a) { | ||
| String name = a.getName(); | ||
| if (!a.isExtern() && !a.isBj() && !a.isNative() && !isFixedEntryPoint(a)) { | ||
| if (!a.isExtern() && !a.isBj() && !a.isNative() | ||
| && !isFixedEntryPoint(a) && !NamePreservation.isPreserved(a)) { | ||
| name = uniqueName(name); | ||
|
Comment on lines
+175
to
176
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.
When a valid Wurst function uses a Lua-only keyword such as AGENTS.md reference: AGENTS.md:L229-L235 Useful? React with 👍 / 👎. |
||
| } else if (isFixedEntryPoint(a)) { | ||
| } else if (isFixedEntryPoint(a) || NamePreservation.isPreserved(a)) { | ||
|
Comment on lines
+174
to
+177
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.
When AGENTS.md reference: AGENTS.md:L229-L235 Useful? React with 👍 / 👎. |
||
| usedNames.add(name); | ||
|
Comment on lines
+177
to
178
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 Lua builds without compressor-driven renaming, if a reachable ordinary function or global has the same IM spelling as a preserved function, the ordinary symbol can claim the name through AGENTS.md reference: AGENTS.md:L229-L235 Useful? React with 👍 / 👎. |
||
| } | ||
|
|
||
|
|
@@ -327,8 +331,8 @@ protected String uniqueName(String rawName) { | |
| } | ||
|
|
||
| public LuaCompilationUnit translate() { | ||
| assertNoDanglingFunctionReferences(prog); | ||
| collectPredefinedNames(); | ||
| assertNoDanglingFunctionReferences(prog); | ||
|
|
||
| normalizeFieldNames(); | ||
|
|
||
|
|
@@ -488,7 +492,8 @@ private boolean isFixedEntryPoint(ImFunction function) { | |
|
|
||
| private void collectPredefinedNames() { | ||
| for (ImFunction function : prog.getFunctions()) { | ||
|
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.
When a class AGENTS.md reference: AGENTS.md:L229-L235 Useful? React with 👍 / 👎. |
||
| if (function.isBj() || function.isExtern() || function.isNative()) { | ||
| if (function.isBj() || function.isExtern() || function.isNative() | ||
| || NamePreservation.isPreserved(function)) { | ||
| // Don't rename Wurst-internal stubs (names starting with __wurst_) | ||
| // since their names are intentionally different from their trace's source name. | ||
| if (!function.getName().startsWith("__wurst_")) { | ||
|
|
@@ -502,6 +507,8 @@ private void collectPredefinedNames() { | |
| if (global.getIsBJ()) { | ||
| setNameFromTrace(global); | ||
| usedNames.add(global.getName()); | ||
| } else if (NamePreservation.isPreserved(global)) { | ||
| usedNames.add(global.getName()); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,7 +5,7 @@ | |
| import de.peeeq.wurstscript.jassIm.*; | ||
| import de.peeeq.wurstscript.translation.imtranslation.ImHelper; | ||
| import de.peeeq.wurstscript.translation.imtranslation.ImTranslator; | ||
| import de.peeeq.wurstscript.validation.TRVEHelper; | ||
| import de.peeeq.wurstscript.validation.NamePreservation; | ||
|
|
||
| import java.util.Collection; | ||
| import java.util.Collections; | ||
|
|
@@ -122,7 +122,7 @@ public static void removeGarbage(ImProg prog, ImTranslator translator) { | |
| Used used = collectUsed(prog, translator); | ||
|
|
||
| prog.getClasses().removeIf(c -> !used.getClasses().contains(c)); | ||
| prog.getGlobals().removeIf(g -> !used.getVars().contains(g) && !TRVEHelper.protectedVariables.contains(g.getName())); | ||
| prog.getGlobals().removeIf(g -> !used.getVars().contains(g) && !NamePreservation.isPreserved(g)); | ||
| prog.getFunctions().removeIf(f -> !used.getFunctions().contains(f)); | ||
| prog.getMethods().removeIf(m -> !used.getMethods().contains(m)); | ||
| for (ImMethod m : prog.getMethods()) { | ||
|
|
@@ -153,7 +153,8 @@ private static Used collectUsed(ImProg prog, ImTranslator translator, | |
| Used used = new Used(translator, ignoredInitializers); | ||
| for (ImFunction f : ImHelper.calculateFunctionsOfProg(prog)) { | ||
| if (f.getName().equals("main") | ||
| || f.getName().equals("config")) { | ||
| || f.getName().equals("config") | ||
| || NamePreservation.isPreserved(f)) { | ||
| visitFunction(f, used); | ||
|
Comment on lines
154
to
158
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.
When AGENTS.md reference: AGENTS.md:L229-L235 Useful? React with 👍 / 👎. |
||
| } | ||
| } | ||
|
|
@@ -221,7 +222,6 @@ private static void visitFunction(ImFunction f, Used used) { | |
| return; | ||
| } | ||
| used.addFunction(f); | ||
|
|
||
| visitType(f.getReturnType(), used); | ||
| f.accept(new Element.DefaultVisitor() { | ||
| @Override | ||
|
|
||
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.
When an
@preserveNamefunction has no IM caller—which is the expected case for a callback invoked solely by Warcraft or other external code—ImOptimizer.removeGarbage()runs before this compressor and retains only functions reachable frommainorconfig; Lua'sRemoveGarbagelikewise ignores the preservation flag. The function is therefore deleted before this check can preserve its name. Add preserved functions as reachability roots in both backend paths so the annotation can expose an external-only entry point.AGENTS.md reference: AGENTS.md:L229-L235
Useful? React with 👍 / 👎.