Skip to content

Fix build non-determinism in J2ObjC code generator. #2807

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.

Sign up for GitHub

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

Open
copybara-service wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix build non-determinism in J2ObjC code generator. #2807

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 2 additions & 2 deletions translator/src/main/java/com/google/devtools/j2objc/gen/ObjectiveCHeaderGenerator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,9 +107,9 @@ protected Set<String> generateFileHeader() {
pushIgnoreDeprecatedDeclarationsPragma();
pushIgnoreNullabilityPragmas();

Set<String> seenTypes = Sets.newHashSet();
Set<String> seenTypes = Sets.newLinkedHashSet();
Set<String> includeFiles = Sets.newTreeSet();
Set<Import> forwardDeclarations = Sets.newHashSet();
Set<Import> forwardDeclarations = Sets.newLinkedHashSet();

includeFiles.add("J2ObjC_header.h");

Expand Down
4 changes: 2 additions & 2 deletions ...lator/src/main/java/com/google/devtools/j2objc/gen/ObjectiveCImplementationGenerator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,8 +149,8 @@ private void printImports() {
print(code);
}

Set<String> seenTypes = Sets.newHashSet();
Set<Import> forwardDecls = Sets.newHashSet();
Set<String> seenTypes = Sets.newLinkedHashSet();
Set<Import> forwardDecls = Sets.newLinkedHashSet();
for (GeneratedType generatedType : getOrderedTypes()) {
String name = generatedType.getTypeName();
seenTypes.add(name);
Expand Down
16 changes: 13 additions & 3 deletions translator/src/main/java/com/google/devtools/j2objc/gen/TypeImplementationGenerator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,11 +90,21 @@ private String getSourceFilePath() {

Path sourceFilePath = toNormalizedSourcePath(sourceFilePathString);
Path cwdPath = toNormalizedSourcePath(".");
if (!sourceFilePath.startsWith(cwdPath)) {
return sourceFilePathString;
if (sourceFilePath.startsWith(cwdPath)) {
return cwdPath.relativize(sourceFilePath).toString();
}

Path tempDir =
Paths.get(System.getProperty("java.io.tmpdir", "/tmp")).normalize().toAbsolutePath();
if (sourceFilePath.startsWith(tempDir)) {
Path relativeToTemp = tempDir.relativize(sourceFilePath);
if (relativeToTemp.getNameCount() > 1) {
return relativeToTemp.subpath(1, relativeToTemp.getNameCount()).toString();
}
return relativeToTemp.toString();
}

return cwdPath.relativize(sourceFilePath).toString();
return sourceFilePathString;
}

protected void generate() {
Expand Down
19 changes: 10 additions & 9 deletions translator/src/main/java/com/google/devtools/j2objc/translate/VariableRenamer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
import com.google.errorprone.annotations.CanIgnoreReturnValue;
import java.util.ArrayDeque;
import java.util.Deque;
import java.util.HashSet;
import java.util.LinkedHashSet;
import java.util.Set;
import javax.lang.model.element.Element;
import javax.lang.model.element.ExecutableElement;
Expand All @@ -52,15 +52,15 @@
public class VariableRenamer extends UnitTreeVisitor {

private final Deque<Set<String>> fieldNameStack = new ArrayDeque<>();
private final Set<TypeElement> renamedTypes = new HashSet<>();
private final Set<TypeElement> renamedTypes = new LinkedHashSet<>();
// Keep track of the variables and names in a scope so we can rename variables in a
// scope-aware manner; start with a sentinel empty scope.
private final Deque<Scope> scopes = new ArrayDeque<>(ImmutableList.of(new Scope()));

// Keep track of variables that are in scope and the names that have already been used.
private static class Scope {
private final Set<VariableElement> variables = new HashSet<>();
private final Set<String> usedNames = new HashSet<>();
private final Set<VariableElement> variables = new LinkedHashSet<>();
private final Set<String> usedNames = new LinkedHashSet<>();

private Scope(Scope enclosingScope) {
this.variables.addAll(enclosingScope.variables);
Expand Down Expand Up @@ -94,12 +94,12 @@ private void collectAndRenameFields(TypeElement type, Set<VariableElement> field
collectAndRenameFields(ElementUtil.getSuperclass(type), fields);
if (!renamedTypes.contains(type)) {
renamedTypes.add(type);
Set<String> superFieldNames = new HashSet<>();
Set<String> superFieldNames = new LinkedHashSet<>();
for (VariableElement superField : fields) {
superFieldNames.add(superField.getSimpleName().toString());
}
// Look for methods that might conflict with a static variable when functionized.
Set<String> staticMethodNames = new HashSet<>();
Set<String> staticMethodNames = new LinkedHashSet<>();
for (ExecutableElement method : ElementUtil.getExecutables(type)) {
if (method.getParameters().size() == 0) {
staticMethodNames.add(nameTable.getFunctionName(method));
Expand Down Expand Up @@ -135,9 +135,9 @@ private void collectAndRenameFields(TypeElement type, Set<VariableElement> field
}

private void pushType(TypeElement type) {
Set<VariableElement> fields = new HashSet<>();
Set<VariableElement> fields = new LinkedHashSet<>();
collectAndRenameFields(type, fields);
Set<String> fullFieldNames = new HashSet<>();
Set<String> fullFieldNames = new LinkedHashSet<>();
for (VariableElement field : fields) {
fullFieldNames.add(nameTable.getVariableShortName(field));
}
Expand Down Expand Up @@ -173,7 +173,8 @@ public void endVisit(SimpleName node) {
}
if (var.getKind().isField()) {
// Make sure fields for the declaring type are renamed.
collectAndRenameFields(ElementUtil.getDeclaringClass(var), new HashSet<VariableElement>());
collectAndRenameFields(
ElementUtil.getDeclaringClass(var), new LinkedHashSet<VariableElement>());
} else {
// Local variable or parameter.
handleVariable(var);
Expand Down
7 changes: 3 additions & 4 deletions translator/src/main/java/com/google/devtools/j2objc/types/HeaderImportCollector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@
import com.google.devtools.j2objc.util.ElementUtil;
import com.google.devtools.j2objc.util.TranslationUtil;
import com.google.devtools.j2objc.util.TypeUtil;
import java.util.HashSet;
import java.util.LinkedHashSet;
import java.util.Set;
import javax.lang.model.element.Element;
Expand Down Expand Up @@ -80,7 +79,7 @@ private boolean include(BodyDeclaration node) {
// Supertypes of the below declared types that haven't been seen by this collector.
private Set<Import> superTypes = new LinkedHashSet<>();
// Declared types seen by this collector.
private Set<Import> declaredTypes = new HashSet<>();
private Set<Import> declaredTypes = new LinkedHashSet<>();
// The current type declarations is annotated for ObjC generics.
private boolean wantsGenerateObjectiveCGenerics = false;
private final boolean includeInnerTypes;
Expand Down Expand Up @@ -152,10 +151,10 @@ public boolean visit(FieldDeclaration node) {
private Set<TypeMirror> objCForwardDeclaredGenericParameters(TypeMirror type) {
// Parameters only needed with ObjC generics are on.
if (!unit.getEnv().options().asObjCGenericDecl() && !wantsGenerateObjectiveCGenerics) {
return new HashSet<>();
return new LinkedHashSet<>();
}

HashSet<TypeMirror> vistedTypes = new HashSet<>();
Set<TypeMirror> vistedTypes = new LinkedHashSet<>();
vistedTypes.add(type);
for (TypeElement bound : unit.getEnv().typeUtil().getObjcUpperBounds(type)) {
vistedTypes.add(bound.asType());
Expand Down