Skip to content

Add SimplifiedPredicate#246

Merged
rcosta358 merged 6 commits into
mainfrom
vc-simplification
Jun 9, 2026
Merged

Add SimplifiedPredicate#246
rcosta358 merged 6 commits into
mainfrom
vc-simplification

Conversation

@rcosta358

@rcosta358 rcosta358 commented Jun 8, 2026

Copy link
Copy Markdown
Collaborator

This PR adds the SimplifiedPredicate that extends the Predicate class and stores the original and simplified predicates and binders.

@rcosta358 rcosta358 self-assigned this Jun 8, 2026
@rcosta358 rcosta358 changed the title Add SimplifiedExpression Add SimplifiedPredicate Jun 8, 2026
@rcosta358 rcosta358 changed the base branch from vcimplication-ordering to main June 9, 2026 11:01

@CatarinaGamboa CatarinaGamboa left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor changes

while (expression instanceof GroupExpression group)
expression = group.getExpression();
while (expression instanceof GroupExpression) {
if (expression instanceof GroupExpression group)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why this if?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need.


public class SimplifiedPredicate extends Predicate {

private final Predicate simplified;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

couldnt simplified be the expression from the super? we can leave it separate if you want but it feels like we could just use the super.expression, no?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, good call.

&& binders.equals(other.binders);
}

public static class Binder {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we nee dthis to be public? if yes create another file with it, but maybe we dont? not sure

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah we need it to be public, and it is currently accessed as SimplifiedPredicate.Binder which I think makes more sense since we don't want to use Binder in any other context, so I think it should stay nested. What do you think?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hum ok i see, we can work with that


private final Predicate simplified;
private final Predicate origin;
private final List<Binder> binders;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

say what you mean by binders


import spoon.reflect.reference.CtTypeReference;

public class SimplifiedPredicate extends Predicate {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually add some javadoc exlaining this class nature and what it represents with an example

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rebase issue? 😕 ?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

@rcosta358 rcosta358 requested a review from CatarinaGamboa June 9, 2026 13:01
@rcosta358 rcosta358 force-pushed the vc-simplification branch from d812c8f to 604bcef Compare June 9, 2026 13:07
@rcosta358 rcosta358 force-pushed the vc-simplification branch from 2a775bf to dfb80ab Compare June 9, 2026 13:45
@rcosta358 rcosta358 added the simplification Related to the simplification of expressions label Jun 9, 2026

@CatarinaGamboa CatarinaGamboa left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@rcosta358 rcosta358 merged commit f2ee10d into main Jun 9, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

simplification Related to the simplification of expressions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants