Skip to content

Commit

Permalink
Automated g4 rollback of changelist 241411023.
Browse files Browse the repository at this point in the history 
*** Reason for rollback ***

Breaks tests

*** Original change description ***

Move hoistExterns after dependency pruning to allow extern files as entry points.

When --experimental_fast mode is turned on for js_lib, it pulls in all of the srcs files as entry_points. However, some of them are externs, which originally are hoisted before dependency pruning. This will cause an error during dependency pruning along the lines of "required entry point <extens file> never provided". In order to unblock experimental_fast, this change moves externs hoisting until after dependency...

***

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=241597920
  • Loading branch information
@EatingW @blickly
EatingW authored and blickly committed Apr 3, 2019
1 parent 2c9cf93 commit 081e13c
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 151 deletions.
26 changes: 10 additions & 16 deletions src/com/google/javascript/jscomp/Compiler.java
View file
Expand Up @@ -1768,14 +1768,9 @@ void orderInputsWithLargeStack() {
}

void orderInputs() {
maybeDoThreadedParsing();

hoistExterns();
// Check if the sources need to be re-ordered.
boolean staleInputs = false;

// Before dependency pruning, save a copy of the original inputs to use for externs hoisting.
ImmutableList<CompilerInput> originalInputs = ImmutableList.copyOf(moduleGraph.getAllInputs());

if (options.getDependencyOptions().needsManagement()) {
for (CompilerInput input : moduleGraph.getAllInputs()) {
// Forward-declare all the provided types, so that they
Expand All @@ -1796,7 +1791,6 @@ void orderInputs() {
MISSING_MODULE_ERROR, e.getMessage()));
}
}
hoistExterns(originalInputs);

// Manage dependencies may move weak sources around, and end up with empty modules.
fillEmptyModules(getModules());
Expand All @@ -1819,7 +1813,7 @@ void orderInputs() {
*/
private void findModulesFromEntryPoints(
boolean supportEs6Modules, boolean supportCommonJSModules) {
maybeDoThreadedParsing();
hoistExterns();
List<CompilerInput> entryPoints = new ArrayList<>();
Map<String, CompilerInput> inputsByProvide = new HashMap<>();
Map<String, CompilerInput> inputsByIdentifier = new HashMap<>();
Expand Down Expand Up @@ -1915,11 +1909,14 @@ private void findModulesFromInput(
}
}

/** Hoists inputs with the @externs annotation into the externs list. */
void hoistExterns(ImmutableList<CompilerInput> originalInputs) {
/**
* Hoists inputs with the @externs annotation into the externs list.
*/
void hoistExterns() {
boolean staleInputs = false;

for (CompilerInput input : originalInputs) {
maybeDoThreadedParsing();
// Iterate a copy because hoisting modifies what we're iterating over.
for (CompilerInput input : ImmutableList.copyOf(moduleGraph.getAllInputs())) {
if (hoistIfExtern(input)) {
staleInputs = true;
}
Expand All @@ -1940,10 +1937,7 @@ private boolean hoistIfExtern(CompilerInput input) {
externsRoot.addChildToBack(input.getAstRoot(this));
input.setIsExtern();

JSModule module = input.getModule();
if (module != null) {
module.remove(input);
}
input.getModule().remove(input);

externs.add(input);
return true;
Expand Down
135 changes: 0 additions & 135 deletions test/com/google/javascript/jscomp/CompilerTest.java
View file
Expand Up @@ -1459,141 +1459,6 @@ public void testEs6ModulePathWithOddCharacters() throws Exception {
assertThat(result.errors).isEmpty();
}

@Test
public void testExternsFileAsEntryPoint() throws Exception {
// Test that you can specify externs as entry points.
// This is needed for turning on experimental_fast for js_lib,
// which adds all sources as entry points.
List<SourceFile> inputs =
ImmutableList.of(
SourceFile.fromCode(
"/externs.js", "/** @fileoverview @externs */ /** @const {number} */ var bar = 1;"),
SourceFile.fromCode("/foo.js", "console.log(0);"));

List<ModuleIdentifier> entryPoints = ImmutableList.of(ModuleIdentifier.forFile("/externs.js"));

CompilerOptions options = createNewFlagBasedOptions();
options.setDependencyOptions(DependencyOptions.pruneForEntryPoints(entryPoints));

List<SourceFile> externs =
AbstractCommandLineRunner.getBuiltinExterns(options.getEnvironment());

Compiler compiler = new Compiler();
compiler.compile(externs, inputs, options);

Result result = compiler.getResult();
assertThat(result.errors).isEmpty();
assertThat(compiler.toSource()).isEqualTo(""); // Empty since srcs are pruned.
}

@Test
public void testExternsFileAsEntryPoint2() throws Exception {
// Test code reference to an extern that doesn't exist,
// but the extern is still the sole entry point.
List<SourceFile> inputs =
ImmutableList.of(
SourceFile.fromCode(
"/externs.js", "/** @fileoverview @externs */ /** @const {number} */ var bar = 1;"),
SourceFile.fromCode("/foo.js", "console.log(nonexistentExtern);"));

List<ModuleIdentifier> entryPoints = ImmutableList.of(ModuleIdentifier.forFile("/externs.js"));

CompilerOptions options = createNewFlagBasedOptions();
options.setDependencyOptions(DependencyOptions.pruneForEntryPoints(entryPoints));

List<SourceFile> externs =
AbstractCommandLineRunner.getBuiltinExterns(options.getEnvironment());

Compiler compiler = new Compiler();
compiler.compile(externs, inputs, options);

Result result = compiler.getResult();
assertThat(result.errors).isEmpty();
assertThat(compiler.toSource()).isEqualTo("");
}

@Test
public void testExternsFileAsEntryPoint3() throws Exception {
// Test code reference to an extern that doesn't exist,
// but the extern and source files are both entry points
List<SourceFile> inputs =
ImmutableList.of(
SourceFile.fromCode(
"/externs.js", "/** @fileoverview @externs */ /** @const {number} */ var bar = 1;"),
SourceFile.fromCode("/foo.js", "console.log(nonexistentExtern);"));

List<ModuleIdentifier> entryPoints =
ImmutableList.of(
ModuleIdentifier.forFile("/externs.js"), ModuleIdentifier.forFile("/foo.js"));

CompilerOptions options = createNewFlagBasedOptions();
options.setDependencyOptions(DependencyOptions.pruneForEntryPoints(entryPoints));

List<SourceFile> externs =
AbstractCommandLineRunner.getBuiltinExterns(options.getEnvironment());

Compiler compiler = new Compiler();
compiler.compile(externs, inputs, options);

Result result = compiler.getResult();
assertThat(result.errors).hasSize(1);
assertThat(result.errors.get(0).getType()).isEqualTo(VarCheck.UNDEFINED_VAR_ERROR);
}

@Test
public void testExternsFileAsEntryPoint4() throws Exception {
// Test that has a code reference to an extern that does exist,
// and the extern and source files are both entry points
List<SourceFile> inputs =
ImmutableList.of(
SourceFile.fromCode(
"/externs.js", "/** @fileoverview @externs */ /** @const {number} */ var bar = 1;"),
SourceFile.fromCode("/foo.js", "console.log(bar);"));

List<ModuleIdentifier> entryPoints =
ImmutableList.of(
ModuleIdentifier.forFile("/externs.js"), ModuleIdentifier.forFile("/foo.js"));

CompilerOptions options = createNewFlagBasedOptions();
options.setDependencyOptions(DependencyOptions.pruneForEntryPoints(entryPoints));

List<SourceFile> externs =
AbstractCommandLineRunner.getBuiltinExterns(options.getEnvironment());

Compiler compiler = new Compiler();
compiler.compile(externs, inputs, options);

Result result = compiler.getResult();
assertThat(result.errors).isEmpty();
assertThat(compiler.toSource()).isEqualTo("console.log(bar);");
}

@Test
public void testExternsFileAsEntryPoint5() throws Exception {
// Test that has a code reference to an extern that does exist,
// and only the source source file is an entry point
List<SourceFile> inputs =
ImmutableList.of(
SourceFile.fromCode(
"/externs.js", "/** @fileoverview @externs */ /** @const {number} */ var bar = 1;"),
SourceFile.fromCode("/foo.js", "console.log(bar);"));

List<ModuleIdentifier> entryPoints = ImmutableList.of(ModuleIdentifier.forFile("/foo.js"));

CompilerOptions options = createNewFlagBasedOptions();
options.setDependencyOptions(DependencyOptions.pruneForEntryPoints(entryPoints));

List<SourceFile> externs =
AbstractCommandLineRunner.getBuiltinExterns(options.getEnvironment());

Compiler compiler = new Compiler();
compiler.compile(externs, inputs, options);

Result result = compiler.getResult();
assertThat(result.errors).isEmpty();
assertThat(compiler.toSource()).isEqualTo("console.log(bar);");
}

@Test
public void testGetEmptyResult() {
Result result = new Compiler().getResult();
Expand Down

0 comments on commit 081e13c

Please sign in to comment.