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

review: fix: Use CtSuperAccess when implicitly accessing superclass field #5406

Merged
merged 5 commits into from
Oct 4, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,9 @@ public static IntExpr getTargetValue(Context context, Map<CtReference, Expr> var
targets.addFirst(target);
target = ((CtTargetedExpression) target).getTarget();
}
targets.addFirst(target);
if (target != null) {
targets.addFirst(target);
}

// Traverse all targets left to right
IntExpr targetValue = null;
Expand All @@ -45,6 +47,9 @@ else if (t instanceof CtArrayRead)
Expr arrayIndex = (Expr) index.getMetadata("value");
targetValue = (IntExpr) memory.readArray((CtArrayTypeReference) arrayRead.getTarget().getType(), targetValue, arrayIndex);
}
else if (t instanceof CtSuperAccess) {
targetValue = context.mkInt(Memory.thisPointer());
}
else if (t instanceof CtVariableRead)
{
targetValue = (IntExpr) variablesMap.get(((CtVariableRead) t).getVariable());
Expand Down
8 changes: 8 additions & 0 deletions spoon-smpl/src/main/java/spoon/smpl/SmPLMethodCFG.java
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,14 @@ private void replace(CtElement e) {
* @param e Element about to be replaced
*/
private void logReplacementWarning(CtElement e) {
if (!e.getPosition().isValidPosition()) {
String code = e.toString();
LoggerFactory.getLogger("spoon-smpl")
.warn("Unsupported element excluded from control flow graph at unknown location: "
+ e.getClass().getSimpleName()
+ code.substring(0, Math.min(40, code.length())));
return;
}
String file = Optional.ofNullable(e.getPosition())
.map(x -> x.getFile())
.map(x -> x.getName())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -987,6 +987,9 @@ public <T> void visitCtThisAccess(CtThisAccess<T> thisAccess) {

@Override
public <T> void visitCtSuperAccess(CtSuperAccess<T> f) {
if (f.isImplicit()) {
return;
}
enterCtExpression(f);
if (f.getTarget() != null) {
scan(f.getTarget());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

import spoon.reflect.code.CtExpression;
import spoon.reflect.code.CtNewClass;
import spoon.reflect.code.CtSuperAccess;
import spoon.reflect.code.CtTargetedExpression;
import spoon.reflect.code.CtThisAccess;
import spoon.reflect.code.CtTypeAccess;
Expand Down Expand Up @@ -96,8 +97,8 @@ protected void handleTargetedExpression(CtTargetedExpression<?, ?> targetedExpre
if (!target.isImplicit()) {
return;
}
if (target instanceof CtThisAccess) {
//the non implicit this access is not forced
if (target instanceof CtThisAccess || target instanceof CtSuperAccess) {
//the implicit this/super access is not forced
return;
}
if (target instanceof CtTypeAccess) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -366,7 +366,18 @@ <T> CtFieldAccess<T> createFieldAccess(SingleNameReference singleNameReference)
if (ref.isStatic() && !ref.getDeclaringType().isAnonymous()) {
va.setTarget(jdtTreeBuilder.getFactory().Code().createTypeAccess(ref.getDeclaringType(), true));
} else if (!ref.isStatic()) {
va.setTarget(jdtTreeBuilder.getFactory().Code().createThisAccess(jdtTreeBuilder.getReferencesBuilder().getTypeReference(singleNameReference.actualReceiverType), true));
TypeBinding fieldDeclarerType = singleNameReference.fieldBinding().declaringClass;
TypeBinding actualReceiverType = singleNameReference.actualReceiverType;
CtTypeReference<?> owningType = jdtTreeBuilder.getReferencesBuilder().getTypeReference(fieldDeclarerType);
if (Arrays.equals(fieldDeclarerType.qualifiedSourceName(), actualReceiverType.qualifiedSourceName())) {
va.setTarget(jdtTreeBuilder.getFactory().Code().createThisAccess(owningType, true));
} else {
va.setTarget(
jdtTreeBuilder.getFactory().createSuperAccess()
.setType(owningType)
.setImplicit(true)
);
}
}
}
return va;
Expand Down
1 change: 1 addition & 0 deletions src/test/java/spoon/reflect/visitor/CtScannerTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
import spoon.metamodel.MetamodelConcept;
import spoon.reflect.code.CtFieldRead;
import spoon.reflect.code.CtInvocation;
import spoon.reflect.code.CtSuperAccess;
SirYwell marked this conversation as resolved.
Show resolved Hide resolved
import spoon.reflect.cu.CompilationUnit;
import spoon.reflect.declaration.CtClass;
import spoon.reflect.declaration.CtElement;
Expand Down
33 changes: 30 additions & 3 deletions src/test/java/spoon/test/targeted/TargetedExpressionTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,7 @@
package spoon.test.targeted;


import java.util.List;

import org.junit.jupiter.api.Test;

import spoon.Launcher;
import spoon.reflect.CtModel;
import spoon.reflect.code.CtExpression;
Expand All @@ -44,6 +41,7 @@
import spoon.reflect.visitor.filter.NamedElementFilter;
import spoon.reflect.visitor.filter.TypeFilter;
import spoon.support.comparator.CtLineElementComparator;
import spoon.support.compiler.VirtualFile;
import spoon.support.reflect.code.CtConstructorCallImpl;
import spoon.support.reflect.code.CtFieldReadImpl;
import spoon.support.reflect.code.CtThisAccessImpl;
Expand All @@ -56,6 +54,8 @@
import spoon.test.targeted.testclasses.Tapas;
import spoon.testing.utils.ModelTest;

import java.util.List;

import static org.hamcrest.CoreMatchers.instanceOf;
import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.MatcherAssert.assertThat;
Expand Down Expand Up @@ -539,6 +539,33 @@ public void testClassDeclaredInALambda() throws Exception {
assertEqualsFieldAccess(new ExpectedTargetedExpression().declaringType(thirdExpectedInner).target(expectedThisAccess).type(CtFieldWrite.class).result("this.index").isLocal(), elements.get(2));
}

@Test
void testUnqualifiedSuperFieldAccess() {
// contract: Unqualified super field accesses are modeled by an implicit CtSuperAccess
Launcher launcher = new Launcher();

launcher.getEnvironment().setComplianceLevel(17);
launcher.addInputResource(new VirtualFile(
"class A {\n" +
" int a;\n" +
"}\n" +
"\n" +
"class B extends A {\n" +
" void foo() {\n" +
" super.a = a;\n" +
" }\n" +
"}\n"
));

CtModel ctModel = launcher.buildModel();

CtFieldRead<?> read = ctModel.getElements(new TypeFilter<>(CtFieldRead.class)).get(0);
assertTrue(read.getTarget() instanceof CtSuperAccess<?>, "Target was no super access");
assertTrue(read.getTarget().isImplicit(), "super target was not implicit");
assertEquals("A", read.getTarget().getType().getQualifiedName());
assertEquals(read.toString(), "a", "super target was still printed");
}

private void assertEqualsFieldAccess(ExpectedTargetedExpression expected, CtFieldAccess<?> fieldAccess) {
if (expected.declaringType == null) {
assertNull(fieldAccess.getVariable().getDeclaringType());
Expand Down
Loading