Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix name collision with generated module names and function local assignments, version 3.2.3 #58

Merged
merged 2 commits into from Apr 2, 2019

Conversation

Projects
None yet
2 participants
@Protryon
Copy link
Member

Protryon commented Apr 2, 2019

The import assignment resolver fails to see generated module names as different from function locals of the same name. To avoid excessive scope analysis throughout execution, all possible name collisions are handled in the variable name generator to avoid this. Fixes #59.

@Protryon Protryon requested a review from michaelficarra Apr 2, 2019

@Protryon Protryon force-pushed the v3.2.3 branch from 473cca7 to eb103ae Apr 2, 2019

@@ -20,18 +20,19 @@ private VariableCollisionResolver() {

public static Tuple3<HashTable<String, Module>, VariableNameGenerator, HashTable<Module, HashTable<String, String>>> resolveCollisions(@Nonnull HashTable<String, Module> modules) {
HashTable<String, GlobalScope> globalScopes = modules.foldLeft((acc, pair) -> acc.put(pair.left, ScopeAnalyzer.analyze(pair.right)), HashTable.emptyUsingIdentity());
HashTable<String, ImmutableSet<String>> names = modules.foldLeft((acc, module) -> GlobalVariableExtractor.extractAllDeclaredVariables(module.right, globalScopes.get(module.left).fromJust(), new ScopeLookup(globalScopes.get(module.left).fromJust())).entries().map(pair -> pair.left).foldLeft((subAcc, name) -> subAcc.put(name, module.left), acc), MultiHashTable.<String, String>emptyUsingEquality()).toHashTable(ImmutableList::uniqByEquality);
HashTable<String, ImmutableSet<String>> globalNames = modules.foldLeft((acc, module) -> GlobalVariableExtractor.extractAllDeclaredVariables(module.right, globalScopes.get(module.left).fromJust(), new ScopeLookup(globalScopes.get(module.left).fromJust())).entries().map(pair -> pair.left).foldLeft((subAcc, name) -> subAcc.put(name, module.left), acc), MultiHashTable.<String, String>emptyUsingEquality()).toHashTable(ImmutableList::uniqByEquality);
HashTable<String, ImmutableSet<String>> allNames = modules.foldLeft((acc, module) -> VariableReferenceExtractor.extractAllReferencedVariables(module.right, globalScopes.get(module.left).fromJust(), new ScopeLookup(globalScopes.get(module.left).fromJust())).entries().map(pair -> pair.left).foldLeft((subAcc, name) -> subAcc.put(name, module.left), acc), MultiHashTable.<String, String>emptyUsingEquality()).toHashTable(ImmutableList::uniqByEquality);

This comment has been minimized.

Copy link
@bakkot

bakkot Apr 2, 2019

Collaborator

It looks like you only need the keys here. If that's so, you shouldn't build the whole hash table.

This comment has been minimized.

Copy link
@bakkot

bakkot Apr 2, 2019

Collaborator

And in fact VariableReferenceExtractor.extractAllReferencedVariables can probably be simplified to just give the strings.

import java.util.List;
import java.util.stream.Collectors;

// determines all declared global variables for a module, linked to a scope analysis variable

This comment has been minimized.

Copy link
@bakkot

bakkot Apr 2, 2019

Collaborator

I think this comment is now wrong.

return extractAllReferencedVariables(module, globalScope, new ScopeLookup(globalScope));
}

private static final class DeclaredVariableReducer extends MonoidalReducer<MultiHashTable<String, Variable>> {

This comment has been minimized.

Copy link
@bakkot

bakkot Apr 2, 2019

Collaborator

Is there a reason to do this with a reducer, instead of just extracting the relevant information from the Scope object directly?

@Protryon Protryon force-pushed the v3.2.3 branch from eb103ae to 1e1054c Apr 2, 2019

@bakkot

bakkot approved these changes Apr 2, 2019

@bakkot bakkot merged commit ce82bc0 into es2017 Apr 2, 2019

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@bakkot bakkot deleted the v3.2.3 branch Apr 2, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.