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.

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

Jump to bottom

Add missing visitElementContent calls #103

Merged
merged 2 commits into from
Aug 3, 2024
Merged

Add missing visitElementContent calls #103

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0bef520
Add missing `visitElementContent` calls
NebelNidas Aug 2, 2024
3404946
Add changes to changelog
NebelNidas Aug 2, 2024
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
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Overhauled the internal `ColumnFileReader` to behave more consistently
- Made handling of the `NEEDS_MULTIPLE_PASSES` flag more consistent, reducing memory usage in a few cases
- Made some internal methods in Enigma and TSRG readers actually private
- Added missing `visitElementContent` calls to CSRG and Recaf Simple readers

## [0.6.1] - 2024-04-15
- Fixed CSRG and JAM writers sometimes skipping elements whose parents have incomplete destination names
Expand Down
2 changes: 2 additions & 0 deletions src/main/java/net/fabricmc/mappingio/format/simple/RecafSimpleFileReader.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,8 +132,10 @@ private static void read(ColumnFileReader reader, String sourceNs, String target
if (visitClass && memberSrcName != null) {
if (!isMethod && visitor.visitField(memberSrcName, memberSrcDesc)) {
visitor.visitDstName(MappedElementKind.FIELD, 0, memberDstName);
visitor.visitElementContent(MappedElementKind.FIELD);
} else if (isMethod && visitor.visitMethod(memberSrcName, memberSrcDesc)) {
visitor.visitDstName(MappedElementKind.METHOD, 0, memberDstName);
visitor.visitElementContent(MappedElementKind.METHOD);
}
}
} while (reader.nextLine(0));
Expand Down
2 changes: 2 additions & 0 deletions src/main/java/net/fabricmc/mappingio/format/srg/TsrgFileReader.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ private static void read(ColumnFileReader reader, String sourceNs, String target

if (visitor.visitMethod(parts[2], parts[4])) {
visitor.visitDstName(MappedElementKind.METHOD, 0, dstName);
visitor.visitElementContent(MappedElementKind.METHOD);
}

continue;
Expand All @@ -155,6 +156,7 @@ private static void read(ColumnFileReader reader, String sourceNs, String target

if (visitor.visitField(parts[2], null)) {
visitor.visitDstName(MappedElementKind.FIELD, 0, dstName);
visitor.visitElementContent(MappedElementKind.FIELD);
}

continue;
Expand Down
34 changes: 24 additions & 10 deletions src/test/java/net/fabricmc/mappingio/VisitOrderVerifyingVisitor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ private void init() {
resetVisitedElementContentUpTo(0);
visitedEnd = false;
lastVisitedElement = null;
visitedLastElement = false;
lastSrcInfo.clear();
}

Expand Down Expand Up @@ -104,6 +105,7 @@ public boolean visitClass(String srcName) throws IOException {
SrcInfo srcInfo = new SrcInfo().srcName(srcName);

assertContentVisited();
assertLastElementContentVisited();
assertNewSrcInfo(elementKind, srcInfo);

visitedClass = true;
Expand All @@ -115,7 +117,7 @@ public boolean visitClass(String srcName) throws IOException {
lastSrcInfo.put(elementKind, srcInfo);
resetVisitedElementContentUpTo(elementKind.level);

return super.visitClass(srcName);
return visitedLastElement = super.visitClass(srcName);
}

@Override
Expand All @@ -126,6 +128,7 @@ public boolean visitField(String srcName, @Nullable String srcDesc) throws IOExc
.srcDesc(srcDesc);

assertClassVisited();
assertLastElementContentVisited();
assertElementContentVisited(elementKind.level - 1);
assertNewSrcInfo(elementKind, srcInfo);

Expand All @@ -137,7 +140,7 @@ public boolean visitField(String srcName, @Nullable String srcDesc) throws IOExc
lastSrcInfo.put(elementKind, srcInfo);
resetVisitedElementContentUpTo(elementKind.level);

return super.visitField(srcName, srcDesc);
return visitedLastElement = super.visitField(srcName, srcDesc);
}

@Override
Expand All @@ -148,6 +151,7 @@ public boolean visitMethod(String srcName, @Nullable String srcDesc) throws IOEx
.srcDesc(srcDesc);

assertClassVisited();
assertLastElementContentVisited();
assertElementContentVisited(elementKind.level - 1);
assertNewSrcInfo(elementKind, srcInfo);

Expand All @@ -159,7 +163,7 @@ public boolean visitMethod(String srcName, @Nullable String srcDesc) throws IOEx
lastSrcInfo.put(elementKind, srcInfo);
resetVisitedElementContentUpTo(elementKind.level);

return super.visitMethod(srcName, srcDesc);
return visitedLastElement = super.visitMethod(srcName, srcDesc);
}

@Override
Expand All @@ -172,6 +176,7 @@ public boolean visitMethodArg(int argPosition, int lvIndex, @Nullable String src

assertFieldNotVisited();
assertMethodVisited();
assertLastElementContentVisited();
assertElementContentVisited(elementKind.level - 1);
assertNewSrcInfo(elementKind, srcInfo);

Expand All @@ -181,7 +186,7 @@ public boolean visitMethodArg(int argPosition, int lvIndex, @Nullable String src
lastSrcInfo.put(elementKind, srcInfo);
resetVisitedElementContentUpTo(elementKind.level);

return super.visitMethodArg(argPosition, lvIndex, srcName);
return visitedLastElement = super.visitMethodArg(argPosition, lvIndex, srcName);
}

@Override
Expand All @@ -196,6 +201,7 @@ public boolean visitMethodVar(int lvtRowIndex, int lvIndex, int startOpIdx, int

assertFieldNotVisited();
assertMethodVisited();
assertLastElementContentVisited();
assertElementContentVisited(elementKind.level - 1);
assertNewSrcInfo(elementKind, srcInfo);

Expand All @@ -205,7 +211,7 @@ public boolean visitMethodVar(int lvtRowIndex, int lvIndex, int startOpIdx, int
lastSrcInfo.put(elementKind, srcInfo);
resetVisitedElementContentUpTo(elementKind.level);

return super.visitMethodVar(lvtRowIndex, lvIndex, startOpIdx, endOpIdx, srcName);
return visitedLastElement = super.visitMethodVar(lvtRowIndex, lvIndex, startOpIdx, endOpIdx, srcName);
}

@Override
Expand All @@ -219,7 +225,7 @@ public void visitDstName(MappedElementKind targetKind, int namespace, String nam
@Override
public void visitDstDesc(MappedElementKind targetKind, int namespace, String desc) throws IOException {
assertElementVisited(targetKind);
assertElementContentNotVisitedUpTo(targetKind.level + 1);
assertElementContentNotVisitedUpTo(targetKind.level + 1); // prevent visitation after visitElementContent of same level

super.visitDstDesc(targetKind, namespace, desc);
}
Expand All @@ -245,15 +251,15 @@ public boolean visitElementContent(MappedElementKind targetKind) throws IOExcept
@Override
public void visitComment(MappedElementKind targetKind, String comment) throws IOException {
assertElementVisited(targetKind);
assertElementContentVisited(targetKind.level);
assertElementContentNotVisitedUpTo(targetKind.level + 1);
assertLastElementContentVisited();

super.visitComment(targetKind, comment);
}

@Override
public boolean visitEnd() throws IOException {
assertContentVisited();
assertLastElementContentVisited();
assertEndNotVisited();

init();
Expand Down Expand Up @@ -375,17 +381,24 @@ private void assertMethodVarVisited() {
}
}

private void assertLastElementContentVisited() {
if (visitedLastElement) {
assertElementContentVisited(lastVisitedElement.level);
assertElementContentNotVisitedUpTo(lastVisitedElement.level + 1);
}
}

private void assertElementContentNotVisitedUpTo(int inclusiveLevel) {
for (int i = visitedElementContent.length - 1; i >= inclusiveLevel; i--) {
if (visitedElementContent[i]) {
throw new IllegalStateException("Element content already visited");
throw new IllegalStateException(lastVisitedElement + " element content already visited");
}
}
}

private void assertElementContentVisited(int depth) {
if (!visitedElementContent[depth]) {
throw new IllegalStateException("Element content not visited");
throw new IllegalStateException(lastVisitedElement + " element content not visited");
}
}

Expand Down Expand Up @@ -413,6 +426,7 @@ private void assertNewSrcInfo(MappedElementKind kind, SrcInfo srcInfo) {
boolean[] visitedElementContent = new boolean[3];
boolean visitedEnd;
MappedElementKind lastVisitedElement;
boolean visitedLastElement;
Map<MappedElementKind, SrcInfo> lastSrcInfo = new HashMap<>();

private static class SrcInfo {
Expand Down
Loading