Fix for entry point -> input matching #1
Conversation
|
Entry points are super tricky. And I really don't ever see the case for an entry point to use the node module resolution algorithm. I would propose the following heuristic for locating an entry point:
|
|
So the case I was attempting to fix with this change can be seen here: https://github.com/gregmagolan/closure-export-example. The read Entry point issue section describes it. Maybe you can think of another way to make this work. Seems like a valid use case to point the |
|
Yeah I think an order of precedence search/matching algorithm for entry points would be good to have. They have never really worked the way I would like, but haven't bubbled up high enough for me to go fix them either. FYI you could make such a PR directly to the main repo. I don't think any other PR in flight will be effected by this. |
|
Agree on the order of precedence search/matching algorithm for entry points. Would a change to the main repo solve the issue with the PR 2641? The input -> entry point matching for the dep tree walking happens in the new code. Specifically: which the patch changes to Are you thinking that a change to the main repo would fix module identifiers that come from |
…(roughly 25% of its previous time). Instead of using the DefinitionUseSiteFinder, as simpler data structure is introduced: OptimizeCalls.ReferenceMap. The ReferenceMap provides a simple list of nodes for every global name and property. This avoids building objects and structures, and identifiers that are unneeded, and avoided redundant analysis. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=173304853
…lues with null and remove them all at the end of the loop. Moves RemoveUnusedVars from #1 to google#7 in the cost rank in samples project builds. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=173314644
*** Reason for rollback *** breakages *** Original change description *** Improve the compile time cost for Optimize Calls from #1 to google#17 (roughly 25% of its previous time). Instead of using the DefinitionUseSiteFinder, as simpler data structure is introduced: OptimizeCalls.ReferenceMap. The ReferenceMap provides a simple list of nodes for every global name and property. This avoids building objects and structures, and identifiers that are unneeded, and avoided redundant analysis. *** ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=173326949
Has the following changes from the original: - fixed bad code, when a symbol definition had a HOOK or other nontrvial expressions (a mismatch between isCandidate and ReferenceMap::getFunctionNodes). - fixes regressions where constructor parameters were not being optimized if used in the target of a property set (f.prototype.method = ...). - fixes regressions where parameters were not optimized if the symbol was used in a typeof/instanceof expression - reverts improvement to constructor optimizations when caused breakage when function constructor properties were used to invoke constructors (until we can decide to ban them or similar). Improve the compile time cost for Optimize Calls from #1 to google#17 (roughly 25% of its previous time). Instead of using the DefinitionUseSiteFinder, as simpler data structure is introduced: OptimizeCalls.ReferenceMap. The ReferenceMap provides... ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=173976261
…r_to_closure Adding instrumentation reporter to closure
With the recent go/accurately-maintain-script-node-featureSet change, we can make the ASTValidator to confirm after each pass that: 1. Features encountered during AST traversal of a SCRIPT <= compiler's allowable featureSet [Source] (becomes optional) 2. Features encountered during AST traversal of a SCRIPT <= that particular SCRIPT node’s FEATURE_SET (accurate, without overestimation during transpile) 3. every SCRIPT node’s FEATURE_SET <= compiler's allowable featureSet (possible to do) #1 and #2 happened automatically as part of go/accurately-maintain-script-node-featureSet. This CL adds #3 above. PiperOrigin-RevId: 554614152
Fix for entry point to input resolution regression on google#2641.
/cc @alexeagle
The issue is reproducible on https://github.com/angular/closure-demo by pointing package.json to a closure compiler built from the google#2641 commit:
In short, the build fails with the current closure configuration in closure-demo repo (closure.conf):
The input
built/src/bootstrap.jsresolves in closure tomodule$$src$$bootstrap(sincebuiltis the js_module_root). With PR2641, it resolves the entry point--entry_point=./built/src/bootstraptomodule$$built$$src$$bootstrap, which doesn't match the input. Without an entry point to analyze the dependency depths, the build completes with an invalid bundle (bundle end ups up being 100 bytes or so).A work-around is possible in the closure conf in closure-demo to make it work:
The closure compiler then correctly matches up the entry point to the input and the build works. Ideally, the build would error out if the entry point does not match an input but that doesn't happen.
With the patch in this PR, the compiler handles both
--entry_point=./built/src/bootstrapand--entry_point=src/bootstrapcorrectly.