Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 14 additions & 5 deletions dev/core/src/com/google/gwt/dev/MinimalRebuildCache.java
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,10 @@ public void clearReboundTypeAssociations(String reboundTypeName) {
* changes in the modified types. For example if the parent of class Foo was changed then the
* castmaps in all children of Foo need to be recreated.
* <p>
* Note that the returned set is not the same as the set of types whose cache was cleared. Cached
* output is cleared for every stale type, while the returned set is narrowed to the stale types
* that are currently reachable, since unreachable types should not be artificially retraversed.
* <p>
* In some ways this process is similar to that performed by the CompilationUnitInvalidator but it
* differs both in what type of cached objects are being cleared (JS versus CompilationUnits) and
* in what invalidation rules must be applied. CompilationUnitInvalidator is concerned only with
Expand Down Expand Up @@ -324,6 +328,15 @@ public Set<String> computeAndClearStaleTypesCache(TreeLogger logger, JTypeOracle
staleTypeNames.removeAll(JProgram.SYNTHETIC_TYPE_NAMES);
}

// Clear the cached output of every stale type, including those that are not currently

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe the Javadoc for the method should make it clear that the set of types that are removed from cache is not the same as the set returned from this method.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point -- the two sets diverged with this change. Added a paragraph in 756c8a6 noting that cached output is cleared for every stale type while the returned set is narrowed to the reachable ones.

// reachable. A stale type that is unreachable now may become reachable again in some later
// compile without being remarked stale, and it must not be reused from cache at that point:
// its cached JS was generated from an old version of its compilation unit and may reference
// (or collide with) global names differently than freshly generated code does.
for (String staleTypeName : staleTypeNames) {
clearCachedTypeOutput(staleTypeName);
}

/*
* Filter for just those stale types that are actually reachable. Since if they're not reachable
* we don't want to artificially traverse them and unnecessarily reveal dependency problems. And
Expand All @@ -337,11 +350,7 @@ public Set<String> computeAndClearStaleTypesCache(TreeLogger logger, JTypeOracle
logger.log(TreeLogger.DEBUG, "known modified types = " + modifiedTypeNames);
logger.log(TreeLogger.DEBUG, "known modified resources = " + modifiedResourcePaths);
logger.log(TreeLogger.DEBUG,
"clearing cached output for resulting stale types = " + staleTypeNames);
}

for (String staleTypeName : staleTypeNames) {
clearCachedTypeOutput(staleTypeName);
"stale types to retraverse = " + staleTypeNames);
}

return Sets.newHashSet(staleTypeNames);
Expand Down
130 changes: 130 additions & 0 deletions dev/core/test/com/google/gwt/dev/CompilerTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
import java.util.List;
import java.util.Set;
import java.util.function.Predicate;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

/**
Expand Down Expand Up @@ -1502,6 +1503,135 @@ public void testIncrementalRecompile_dateStampChange() throws UnableToCompleteEx
checkIncrementalRecompile_dateStampChange(JsOutputOption.DETAILED);
}

// Repro for issue #9565. A stale type that is not currently reachable must still have its
// cached JS cleared; otherwise, when it becomes reachable again in a later compile, its
// outdated JS is reused and can reference (or collide with) global names inconsistently with
// freshly generated code.
public void testIncrementalRecompile_unreachableStaleTypeRegainsReachability()
throws UnableToCompleteException, IOException, InterruptedException {
// Uses PRETTY output so that synthetic lambda type names are recognizable in the JS.
JsOutputOption output = JsOutputOption.PRETTY;

MockJavaResource widgetsWithThreeLambdas =
JavaResourceBase.createMockJavaResource("com.foo.Widgets",
"""
package com.foo;
public class Widgets {
interface IntFilter {
boolean test(int value);
}
public static int state;
public static void ping() {
state++;
}
public static void trigger() {
Runnable a = () -> state++;
int cursorRow = 10;
IntFilter filter = value -> value >= cursorRow;
Runnable c = () -> state--;
a.run();
if (filter.test(11)) {
c.run();
}
}
}
""");
// The same type with an extra lambda inserted first, so that the synthetic lambda types of
// trigger() are renumbered and the name 'Widgets$lambda$1$Type' etc. now denote different
// lambdas than before.
MockJavaResource widgetsWithFourLambdas =
JavaResourceBase.createMockJavaResource("com.foo.Widgets",
"""
package com.foo;
public class Widgets {
interface IntFilter {
boolean test(int value);
}
public static int state;
public static void ping() {
state++;
}
public static void trigger() {
Runnable z = () -> state = state + 12345;
z.run();
Runnable a = () -> state++;
int cursorRow = 10;
IntFilter filter = value -> value >= cursorRow;
Runnable c = () -> state--;
a.run();
if (filter.test(11)) {
c.run();
}
}
}
""");
MockJavaResource entryPointCallingTrigger =
JavaResourceBase.createMockJavaResource("com.foo.TestEntryPoint",
"""
package com.foo;
import com.google.gwt.core.client.EntryPoint;
public class TestEntryPoint implements EntryPoint {
@Override
public void onModuleLoad() {
Widgets.ping();
Widgets.trigger();
}
}
""");
MockJavaResource entryPointNotCallingTrigger =
JavaResourceBase.createMockJavaResource("com.foo.TestEntryPoint",
"""
package com.foo;
import com.google.gwt.core.client.EntryPoint;
public class TestEntryPoint implements EntryPoint {
@Override
public void onModuleLoad() {
Widgets.ping();
}
}
""");

MinimalRebuildCache relinkMinimalRebuildCache = new MinimalRebuildCache();
File relinkApplicationDir = createTempDir();

// Compile the app so that the lambda types in Widgets.trigger() are reachable and their JS is
// cached.
compileToJs(relinkApplicationDir, "com.foo.SimpleModule", Lists.newArrayList(
simpleModuleResource, entryPointCallingTrigger, widgetsWithThreeLambdas),
relinkMinimalRebuildCache, null, output);

// Stop calling trigger(). Its lambda types become unreachable, but since Widgets was not
// modified their cached JS is retained.
compileToJs(relinkApplicationDir, "com.foo.SimpleModule",
Lists.<MockResource> newArrayList(entryPointNotCallingTrigger), relinkMinimalRebuildCache,
null, output);

// Call trigger() again and modify Widgets so that its lambda types are renumbered. The lambda
// types are stale but were unreachable in the previous compile; their outdated cached JS must
// not leak into the output.
String relinkedJs = compileToJs(relinkApplicationDir, "com.foo.SimpleModule",
Lists.<MockResource> newArrayList(entryPointCallingTrigger, widgetsWithFourLambdas),
relinkMinimalRebuildCache, null, output);

// The output must contain the current version of trigger(), not the cached stale one.
assertTrue("expected the regenerated trigger() body to be present in the output",
relinkedJs.contains("12345"));

// Every referenced lambda type constructor must be defined in the output. A dangling
// reference indicates that a stale, differently-named version of the type was reused.
Matcher useMatcher =
Pattern.compile("new\\s+([\\w$]*Widgets\\$lambda\\$\\d+\\$Type[\\w$]*)\\(")
.matcher(relinkedJs);
int lambdaCtorUses = 0;
while (useMatcher.find()) {
lambdaCtorUses++;
String usedCtorName = useMatcher.group(1);
assertTrue("lambda type constructor " + usedCtorName + " is referenced but never defined",
relinkedJs.contains("function " + usedCtorName + "("));
}
assertTrue("expected at least one lambda type constructor reference", lambdaCtorUses > 0);
}

// Repro for bug #9518
public void testIncrementalRecompile_jsPropertyConsistencyCheck()
throws UnableToCompleteException,
Expand Down