Skip to content

Commit cfb1c5d

Browse files
FrottyFrotty
authored
Fix module resolution bug (#1104)
- Invalidate caches before each validation - Don't throw errors during a Phase where modules haven't been expanded yet and their types are still the synthetic module type and not the actual type
1 parent 76c4fc9 commit cfb1c5d

10 files changed

Lines changed: 495 additions & 59 deletions

File tree

‎de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/languageserver/ModelManagerImpl.java‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -629,7 +629,6 @@ private void doTypeCheckPartial(WurstGui gui, List<WFile> toCheckFilenames, Set<
629629

630630
@Override
631631
public void reconcile(Changes changes) {
632-
GlobalCaches.clearAll();
633632
WurstModel model2 = model;
634633
if (model2 == null) {
635634
return;

‎de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/attributes/AttrModuleInstanciations.java‎

Lines changed: 24 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -9,33 +9,37 @@ public final class AttrModuleInstanciations {
99
private AttrModuleInstanciations() {}
1010

1111
public static @Nullable ModuleDef getModuleOrigin(ModuleInstanciation mi) {
12-
// NOTE: For ModuleInstanciation the "name" used for resolution has historically been getName().
13-
// Keep this to preserve prior behavior.
12+
// A ModuleInstanciation's parent is always ModuleInstanciations (the list),
13+
// whose parent is either a ClassDef or a ModuleDef.
14+
// We must resolve the module name through that owner's scope.
15+
1416
final String name = mi.getName();
1517

16-
// 1) Normal path: resolve relative to the lexical parent (old behavior)
17-
final Element parent = mi.getParent();
18-
if (parent != null) {
19-
TypeDef def = parent.lookupType(name, /*showErrors*/ false);
20-
if (def instanceof ModuleDef) {
21-
return (ModuleDef) def;
22-
}
23-
// Attached but not found -> keep the old error
24-
mi.addError("Could not find module origin for " + Utils.printElement(mi));
18+
Element parent = mi.getParent(); // This is ModuleInstanciations (plural)
19+
if (parent == null) {
20+
// Detached node during incremental compilation - this is transient.
21+
// Don't emit errors; return null and let the next full pass resolve it.
22+
return null;
23+
}
24+
25+
// Get the actual owner (ClassDef or ModuleDef)
26+
Element owner = parent.getParent();
27+
if (owner == null) {
28+
// Still detached at the owner level
2529
return null;
2630
}
2731

28-
// 2) Detached during incremental build: try the nearest attached scope
29-
final WScope scope = mi.attrNearestScope();
30-
if (scope != null) {
31-
TypeDef def = scope.lookupType(name, /*showErrors*/ false);
32-
if (def instanceof ModuleDef) {
33-
return (ModuleDef) def;
34-
}
32+
// Resolve through the owner's scope
33+
TypeDef def = owner.lookupType(name, /*showErrors*/ false);
34+
if (def instanceof ModuleDef) {
35+
return (ModuleDef) def;
36+
}
37+
38+
// Only emit error if we're fully attached (not in a transient state)
39+
if (mi.getModel() != null) {
40+
mi.addError("Could not find module origin for " + Utils.printElement(mi));
3541
}
3642

37-
// 3) Still not found and we're detached: this can be a transient state,
38-
// so don't emit an error here. Return null and let callers handle gracefully.
3943
return null;
4044
}
4145
}

‎de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/attributes/AttrPossibleFunctionSignatures.java‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -179,11 +179,14 @@ public static ImmutableCollection<FunctionSignature> calculate(ExprMemberMethod
179179
if (recv != null) {
180180
VariableBinding m = leftType.matchAgainstSupertype(
181181
recv, mm, sig.getMapping(), VariablePosition.RIGHT);
182-
if (m == null) {
183-
// Should not happen; lookupMemberFuncs already checked. Skip defensively.
184-
continue;
182+
183+
// IMPORTANT:
184+
// For members injected via `use module`, the receiver can be a synthetic/module `thistype`
185+
// that is not directly comparable here (especially during incremental builds).
186+
// Do NOT drop the candidate if binding fails; keep it and let arg matching rank it later.
187+
if (m != null) {
188+
sig = sig.setTypeArgs(mm, m);
185189
}
186-
sig = sig.setTypeArgs(mm, m);
187190
}
188191

189192
// Apply explicit type args from the call-site (e.g., c.foo<T,...>(...))

‎de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/types/WurstType.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import de.peeeq.wurstscript.jassIm.ImExprOpt;
66
import de.peeeq.wurstscript.jassIm.ImType;
77
import de.peeeq.wurstscript.translation.imtranslation.ImTranslator;
8+
import de.peeeq.wurstscript.validation.GlobalCaches;
89
import io.vavr.control.Option;
910
import org.eclipse.jdt.annotation.Nullable;
1011

‎de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/validation/GlobalCaches.java‎

Lines changed: 132 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,57 @@
11
package de.peeeq.wurstscript.validation;
22

33
import de.peeeq.wurstscript.ast.Element;
4-
import de.peeeq.wurstscript.attributes.names.NameResolution;
54
import de.peeeq.wurstscript.intermediatelang.ILconst;
65
import de.peeeq.wurstscript.intermediatelang.interpreter.LocalState;
7-
import de.peeeq.wurstscript.types.WurstType;
86
import it.unimi.dsi.fastutil.objects.Object2ObjectOpenHashMap;
9-
import it.unimi.dsi.fastutil.objects.Reference2BooleanOpenHashMap;
10-
import it.unimi.dsi.fastutil.objects.Reference2ObjectOpenHashMap;
117

128
import java.util.Arrays;
139
import java.util.Map;
14-
import java.util.WeakHashMap;
10+
import java.util.concurrent.atomic.AtomicLong;
1511

1612
// Expose static fields only if you already have them there; otherwise, just clear via dedicated methods.
1713
public final class GlobalCaches {
14+
15+
// Statistics tracking
16+
public static class CacheStats {
17+
final AtomicLong hits = new AtomicLong();
18+
final AtomicLong misses = new AtomicLong();
19+
final AtomicLong evictions = new AtomicLong();
20+
final String name;
21+
22+
CacheStats(String name) {
23+
this.name = name;
24+
}
25+
26+
void recordHit() {
27+
hits.incrementAndGet();
28+
}
29+
30+
void recordMiss() {
31+
misses.incrementAndGet();
32+
}
33+
34+
void recordEviction(int count) {
35+
evictions.addAndGet(count);
36+
}
37+
38+
double hitRate() {
39+
long h = hits.get();
40+
long m = misses.get();
41+
long total = h + m;
42+
return total == 0 ? 0.0 : (double) h / total;
43+
}
44+
45+
@Override
46+
public String toString() {
47+
return String.format("%s: hits=%d, misses=%d, hitRate=%.2f%%, evictions=%d",
48+
name, hits.get(), misses.get(), hitRate() * 100, evictions.get());
49+
}
50+
}
51+
52+
private static final CacheStats lookupStats = new CacheStats("LookupCache");
53+
private static final CacheStats localStateStats = new CacheStats("LocalStateCache");
54+
1855
// Optimized ArgumentKey that minimizes allocation overhead
1956
public static final class ArgumentKey {
2057
private final ILconst[] args;
@@ -48,21 +85,48 @@ public boolean equals(Object o) {
4885
}
4986
}
5087

51-
public enum Mode { TEST_ISOLATED, DEV_PERSISTENT }
88+
public enum Mode {TEST_ISOLATED, DEV_PERSISTENT}
89+
5290
private static volatile Mode mode = Mode.DEV_PERSISTENT;
5391

54-
public static void setMode(Mode m) { mode = m; }
55-
public static Mode mode() { return mode; }
92+
public static void setMode(Mode m) {
93+
mode = m;
94+
}
5695

57-
private GlobalCaches() {}
96+
public static Mode mode() {
97+
return mode;
98+
}
5899

59-
public static final Object2ObjectOpenHashMap<Object, Object2ObjectOpenHashMap<ArgumentKey, LocalState>> LOCAL_STATE_CACHE = new Object2ObjectOpenHashMap<>();
60-
public static final Reference2ObjectOpenHashMap<WurstType, Reference2BooleanOpenHashMap<WurstType>> SUBTYPE_MEMO = new Reference2ObjectOpenHashMap<>();
100+
private GlobalCaches() {
101+
}
61102

62-
/** Call this between tests (and after each compile) */
103+
// Wrapped caches with statistics
104+
public static final Object2ObjectOpenHashMap<Object, Object2ObjectOpenHashMap<ArgumentKey, LocalState>> LOCAL_STATE_CACHE =
105+
new Object2ObjectOpenHashMap<Object, Object2ObjectOpenHashMap<ArgumentKey, LocalState>>() {
106+
@Override
107+
public Object2ObjectOpenHashMap<ArgumentKey, LocalState> get(Object key) {
108+
Object2ObjectOpenHashMap<ArgumentKey, LocalState> result = super.get(key);
109+
if (result != null) {
110+
localStateStats.recordHit();
111+
} else {
112+
localStateStats.recordMiss();
113+
}
114+
return result;
115+
}
116+
117+
@Override
118+
public void clear() {
119+
localStateStats.recordEviction(size());
120+
super.clear();
121+
}
122+
};
123+
124+
125+
/**
126+
* Call this between tests (and after each compile)
127+
*/
63128
public static void clearAll() {
64129
LOCAL_STATE_CACHE.clear();
65-
SUBTYPE_MEMO.clear();
66130
lookupCache.clear();
67131
}
68132

@@ -95,5 +159,59 @@ public int hashCode() {
95159
}
96160
}
97161

98-
public static final Map<CacheKey, Object> lookupCache = new Object2ObjectOpenHashMap<>();
162+
public static final Map<CacheKey, Object> lookupCache = new Object2ObjectOpenHashMap<CacheKey, Object>() {
163+
@Override
164+
public Object get(Object key) {
165+
Object result = super.get(key);
166+
if (result != null) {
167+
lookupStats.recordHit();
168+
} else {
169+
lookupStats.recordMiss();
170+
}
171+
return result;
172+
}
173+
174+
@Override
175+
public Object put(CacheKey key, Object value) {
176+
// Note: put returns old value, null if new entry
177+
Object old = super.put(key, value);
178+
if (old == null) {
179+
// New entry, the miss was already recorded in get()
180+
}
181+
return old;
182+
}
183+
184+
@Override
185+
public void clear() {
186+
lookupStats.recordEviction(size());
187+
super.clear();
188+
}
189+
};
190+
191+
// Statistics methods
192+
public static void printStats() {
193+
System.out.println("=== GlobalCaches Statistics ===");
194+
System.out.println(lookupStats);
195+
System.out.println(localStateStats);
196+
System.out.println("Current sizes: lookup=" + lookupCache.size() +
197+
", localState=" + LOCAL_STATE_CACHE.size());
198+
System.out.println("==============================");
199+
}
200+
201+
public static void resetStats() {
202+
lookupStats.hits.set(0);
203+
lookupStats.misses.set(0);
204+
lookupStats.evictions.set(0);
205+
localStateStats.hits.set(0);
206+
localStateStats.misses.set(0);
207+
localStateStats.evictions.set(0);
208+
}
209+
210+
public static CacheStats getLookupStats() {
211+
return lookupStats;
212+
}
213+
214+
public static CacheStats getLocalStateStats() {
215+
return localStateStats;
216+
}
99217
}

‎de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/validation/WurstValidator.java‎

Lines changed: 4 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
import java.util.stream.Collectors;
2424

2525
import static de.peeeq.wurstscript.attributes.SmallHelpers.superArgs;
26-
import static de.peeeq.wurstscript.validation.GlobalCaches.SUBTYPE_MEMO;
2726

2827
/**
2928
* this class validates a wurstscript program
@@ -65,7 +64,7 @@ public void validate(Collection<CompilationUnit> toCheck) {
6564
visitedFunctions = 0;
6665
heavyFunctions.clear();
6766
heavyBlocks.clear();
68-
SUBTYPE_MEMO.clear();
67+
GlobalCaches.clearAll();
6968

7069
lightValidation(toCheck);
7170

@@ -1614,26 +1613,12 @@ private static boolean isStrictSuperclassOf(ClassDef sup, ClassDef sub) {
16141613
return false;
16151614
}
16161615

1617-
private static boolean isSubtypeCached(WurstType actual, WurstType expected, Annotation site) {
1616+
private static boolean isSubtypeCached(WurstType actual, WurstType expected, Element site) {
1617+
// Fast paths first
16181618
if (actual == expected) return true;
1619-
// quick structural equality before expensive check
16201619
if (actual.equalsType(expected, site)) return true;
16211620

1622-
Reference2BooleanOpenHashMap<WurstType> inner = SUBTYPE_MEMO.get(actual);
1623-
if (inner != null && inner.containsKey(expected)) {
1624-
return inner.getBoolean(expected);
1625-
}
1626-
1627-
boolean res = actual.isSubtypeOf(expected, site);
1628-
1629-
if (inner == null) {
1630-
inner = new Reference2BooleanOpenHashMap<>();
1631-
SUBTYPE_MEMO.put(actual, inner);
1632-
}
1633-
if (!inner.containsKey(expected)) {
1634-
inner.put(expected, res);
1635-
}
1636-
return res;
1621+
return actual.isSubtypeOf(expected, site);
16371622
}
16381623

16391624
private void checkAnnotation(Annotation a) {

0 commit comments

Comments
 (0)