Skip to content

Commit

Permalink
Check more carefully whether a weak module already exists and test th…
Browse files Browse the repository at this point in the history 
…at the weak module invariants are enforced.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=223606496
  • Loading branch information
@tjgq
tjgq committed Dec 4, 2018
1 parent 36c416d commit 0f952c4
Show file tree
Hide file tree
Showing 2 changed files with 153 additions and 35 deletions.
90 changes: 55 additions & 35 deletions src/com/google/javascript/jscomp/Compiler.java
View file
Expand Up @@ -567,49 +567,69 @@ private void checkFirstModule(List<JSModule> modules) {
}
}

/** Moves all weak sources into a separate weak module that depends on every other module. */
private List<JSModule> moveWeakSources(List<JSModule> modules) {
// Collect weak sources.
List<CompilerInput> weakInputs = new ArrayList<>();
/**
* Returns a new module list where all weak sources have been moved into a separate weak module
* that depends on every other module.
*
* <p>The weak module may already exist when calling the compiler through the Java API, e.g. when
* restoring state for a multi-stage compilation. In this case, this method asserts that the weak
* module contains exactly the weak sources and depends on every other module.
*
* @throws RuntimeException if the weak module already exists but it does not respect the weak
* module invariants.
*/
private static List<JSModule> moveWeakSources(List<JSModule> modules) {
// Try to find an existing weak module, and collect weak sources belonging to strong modules.
JSModule weakModule = null;
List<CompilerInput> weakInputsToMove = new ArrayList<>();
for (JSModule module : modules) {
if (module.getName().equals(JSModule.WEAK_MODULE_NAME)) {
// Skip an already existing weak module - see below.
continue;
}
for (int i = 0; i < module.getInputCount(); ) {
CompilerInput input = module.getInput(i);
if (input.getSourceFile().isWeak()) {
module.remove(input);
weakInputs.add(input);
} else {
i++;
weakModule = module;
} else {
for (int i = 0; i < module.getInputCount(); ) {
CompilerInput input = module.getInput(i);
if (input.getSourceFile().isWeak()) {
module.remove(input);
weakInputsToMove.add(input);
} else {
i++;
}
}
}
}

// If a weak module already exists (e.g. in a stage 2 compilation), make sure it contains all
// weak sources, but leave the module graph otherwise untouched.
if (moduleGraph != null
&& moduleGraph.getModuleByName(JSModule.WEAK_MODULE_NAME) != null
&& !weakInputs.isEmpty()) {
throw new RuntimeException(
"A weak module already exists but weak sources were found in other modules.");
}

// Create the weak module and make it depend on every other module.
JSModule weakModule = new JSModule(JSModule.WEAK_MODULE_NAME);
for (JSModule module : modules) {
weakModule.addDependency(module);
}

// Move the weak sources.
for (CompilerInput input : weakInputs) {
weakModule.add(input);
if (weakModule != null) {
// The weak module already exists. Check the invariants.
if (!weakInputsToMove.isEmpty()) {
throw new RuntimeException(
"A weak module already exists but weak sources were found in other modules.");
}
for (CompilerInput input : weakModule.getInputs()) {
if (!input.getSourceFile().isWeak()) {
throw new RuntimeException(
"A weak module already exists but strong sources were found in it.");
}
}
ImmutableSet<JSModule> deps = ImmutableSet.copyOf(weakModule.getDependencies());
for (JSModule module : modules) {
if (!module.equals(weakModule) && !deps.contains(module)) {
throw new RuntimeException(
"A weak module already exists but it does not depend on every other module.");
}
}
} else {
// The weak module does not exist yet. Create it and move the weak sources into it.
weakModule = new JSModule(JSModule.WEAK_MODULE_NAME);
for (JSModule module : modules) {
weakModule.addDependency(module);
}
for (CompilerInput input : weakInputsToMove) {
weakModule.add(input);
}
// Make a copy in case the original list is immutable.
modules = ImmutableList.<JSModule>builder().addAll(modules).add(weakModule).build();
}

// Make a copy in case the original list is immutable.
modules = ImmutableList.<JSModule>builder().addAll(modules).add(weakModule).build();

return modules;
}

Expand Down
98 changes: 98 additions & 0 deletions test/com/google/javascript/jscomp/CompilerTest.java
View file
Expand Up @@ -19,6 +19,7 @@
import static com.google.common.truth.Truth.assertWithMessage;
import static com.google.javascript.jscomp.CompilerTestCase.lines;
import static java.nio.charset.StandardCharsets.UTF_8;
import static org.junit.Assert.fail;

import com.google.common.base.Joiner;
import com.google.common.collect.ImmutableList;
Expand Down Expand Up @@ -2077,4 +2078,101 @@ public void testWeakSourcesEntryPoint() throws Exception {
assertThat(compiler.toSource())
.isEqualTo("var module$exports$strong={};function module$contents$strong_f(x){alert(x)};");
}

@Test
public void testPreexistingWeakModule() throws Exception {
JSModule strong = new JSModule("m");
strong.add(SourceFile.fromCode("strong.js", "goog.provide('a');", SourceKind.STRONG));
JSModule weak = new JSModule(JSModule.WEAK_MODULE_NAME);
weak.add(SourceFile.fromCode("weak.js", "goog.provide('b');", SourceKind.WEAK));
weak.addDependency(strong);

CompilerOptions options = new CompilerOptions();
options.setEmitUseStrict(false);
options.setClosurePass(true);

Compiler compiler = new Compiler();

compiler.initModules(ImmutableList.of(), ImmutableList.of(strong, weak), options);

compiler.parse();
compiler.check();
compiler.performOptimizations();

assertThat(compiler.getModuleGraph().getModuleCount()).isEqualTo(2);
assertThat(Iterables.get(compiler.getModuleGraph().getAllModules(), 0).getName())
.isEqualTo("m");
assertThat(Iterables.get(compiler.getModuleGraph().getAllModules(), 1).getName())
.isEqualTo(JSModule.WEAK_MODULE_NAME);

assertThat(compiler.toSource()).isEqualTo("var a={};");
}

@Test
public void testPreexistingWeakModuleWithAdditionalStrongSources() throws Exception {
JSModule strong = new JSModule("m");
strong.add(SourceFile.fromCode("strong.js", "goog.provide('a');", SourceKind.STRONG));
JSModule weak = new JSModule(JSModule.WEAK_MODULE_NAME);
weak.add(SourceFile.fromCode("weak.js", "goog.provide('b');", SourceKind.WEAK));
weak.add(
SourceFile.fromCode(
"weak_but_actually_strong.js", "goog.provide('c');", SourceKind.STRONG));
weak.addDependency(strong);

CompilerOptions options = new CompilerOptions();
Compiler compiler = new Compiler();

try {
compiler.initModules(ImmutableList.of(), ImmutableList.of(strong, weak), options);
fail();
} catch (RuntimeException e) {
assertThat(e)
.hasMessageThat()
.isEqualTo("A weak module already exists but strong sources were found in it.");
}
}

@Test
public void testPreexistingWeakModuleWithMissingWeakSources() throws Exception {
JSModule strong = new JSModule("m");
strong.add(SourceFile.fromCode("strong.js", "goog.provide('a');", SourceKind.STRONG));
strong.add(
SourceFile.fromCode("strong_but_actually_weak.js", "goog.provide('b');", SourceKind.WEAK));
JSModule weak = new JSModule(JSModule.WEAK_MODULE_NAME);
weak.add(SourceFile.fromCode("weak.js", "goog.provide('c');", SourceKind.WEAK));
weak.addDependency(strong);

CompilerOptions options = new CompilerOptions();
Compiler compiler = new Compiler();

try {
compiler.initModules(ImmutableList.of(), ImmutableList.of(strong, weak), options);
fail();
} catch (RuntimeException e) {

assertThat(e)
.hasMessageThat()
.isEqualTo("A weak module already exists but weak sources were found in other modules.");
}
}

@Test
public void testPreexistingWeakModuleWithIncorrectDependencies() throws Exception {
JSModule m1 = new JSModule("m1");
JSModule m2 = new JSModule("m2");
JSModule weak = new JSModule(JSModule.WEAK_MODULE_NAME);
weak.addDependency(m1);

CompilerOptions options = new CompilerOptions();
Compiler compiler = new Compiler();

try {
compiler.initModules(ImmutableList.of(), ImmutableList.of(m1, m2, weak), options);
fail();
} catch (RuntimeException e) {
assertThat(e)
.hasMessageThat()
.isEqualTo("A weak module already exists but it does not depend on every other module.");
}
}
}

0 comments on commit 0f952c4

Please sign in to comment.