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

[Switch Restructuring] Insert CaseNodes #257

Merged
merged 12 commits into from
Jul 4, 2023
Merged

[Switch Restructuring] Insert CaseNodes #257

merged 12 commits into from
Jul 4, 2023

Conversation

github-actions[bot]
Copy link
Contributor

closes #20

@github-actions github-actions bot added feature-request New feature or request priority-low Low priority issue labels Jun 20, 2023
@ebehner ebehner force-pushed the issue-20-_Switch_Restructuring_Insert_CaseNodes branch 2 times, most recently from 0d7cd32 to b4c110e Compare June 27, 2023 15:13
@ebehner ebehner force-pushed the issue-20-_Switch_Restructuring_Insert_CaseNodes branch from b4c110e to fac0f4e Compare June 28, 2023 12:21
@ebehner ebehner marked this pull request as ready for review June 29, 2023 07:00
@ebehner ebehner requested a review from steffenenders June 29, 2023 07:00
steffenenders
steffenenders reviewed Jun 29, 2023
View reviewed changes
.../restructuring_commons/condition_aware_refinement_commons/initial_switch_node_constructer.py Outdated
@@ -197,6 +200,15 @@ def _get_possible_case_candidate_for(self, ast_node: AbstractSyntaxTreeNode) ->

return None

def _clean_up_reachability(self, possible_switch_node: SwitchNodeCandidate, sibling_reachability):
Copy link
Member

Choose a reason for hiding this comment

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

You are missing a cod string here.

Copy link
Collaborator

Choose a reason for hiding this comment

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

added

.../restructuring_commons/condition_aware_refinement_commons/initial_switch_node_constructer.py Outdated
@@ -348,45 +364,26 @@ def _update_reaching_condition_of(self, case_node: CaseNode, considered_conditio
:param case_node: The case node where we want to update the reaching condition.
:param considered_conditions: The conditions (literals) that are already fulfilled when we reach the given case node.
"""
literals_of_case_node = set(case_node.reaching_condition.get_literals())
# literals_of_case_node = set(case_node.reaching_condition.get_literals())
Copy link
Member

Choose a reason for hiding this comment

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

Remove

Copy link
Collaborator

Choose a reason for hiding this comment

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

done

.../restructuring_commons/condition_aware_refinement_commons/initial_switch_node_constructer.py Outdated
literals_of_case_node.remove(literal_of_case)
for constant, literal in considered_conditions.items():
if constant in constant_of_case_node_literal:
del constant_of_case_node_literal[constant]
Copy link
Member

Choose a reason for hiding this comment

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

Maybe use dict.pop(<key>) instead? :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

changed, but here I think it makes no difference

.../restructuring_commons/condition_aware_refinement_commons/initial_switch_node_constructer.py Outdated
condition_for_constant: Dict[Constant, LogicCondition] = {
c: l for l, c in self.asforest.switch_node_handler.get_literal_and_constant_for(case.reaching_condition)
}
if None in condition_for_constant:
Copy link
Member

Choose a reason for hiding this comment

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

😬

Copy link
Collaborator

Choose a reason for hiding this comment

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

changed it

...ing_commons/condition_aware_refinement_commons/missing_case_finder_intersecting_constants.py Outdated
if (intersecting_linear_case := self.__get_linear_order_intersection_constants(intersection_cases)) is None:
return
compare_node = possible_case.get_head
# Insert content before case-node
Copy link
Member

Choose a reason for hiding this comment

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

How come you didn't divide this into smaller functions?

Copy link
Member

Choose a reason for hiding this comment

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

Your three comments suggest that you have three behaviours that could be their own functions.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I partially extracted functions, hope this makes it better

...ing_commons/condition_aware_refinement_commons/missing_case_finder_intersecting_constants.py Outdated
self.__resort_cases(common_cases + uncommon_cases, old_children_order)
return common_cases[-1]

def _add_case_behind(self, intersecting_linear_case: Tuple[CaseNode], possible_case_properties: IntersectingCaseNodeProperties):
Copy link
Member

Choose a reason for hiding this comment

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

*_add_case_after

Copy link
Member

Choose a reason for hiding this comment

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

Also, add doc-string

Copy link
Collaborator

Choose a reason for hiding this comment

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

done

...ing_commons/condition_aware_refinement_commons/missing_case_finder_intersecting_constants.py
if common_cases or uncommon_cases:
yield common_cases, uncommon_cases

def __resort_cases(self, new_case_order: List[CaseNode], old_case_children: List[AbstractSyntaxTreeNode]):
Copy link
Member

Choose a reason for hiding this comment

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

doc-string

Copy link
Collaborator

Choose a reason for hiding this comment

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

done

decompiler/structures/ast/syntaxforest.py Outdated
assert (parent := self.have_same_parent(combinable_switch_nodes)) is not None, "All switch nodes must have the same parent."
assert isinstance(parent, SeqNode), "The parent of all switches must be a Sequence node."
self._add_node(new_switch_node := self.factory.create_switch_node(combinable_switch_nodes[0].expression))
self._add_edge(parent, new_switch_node)
switch_node_cases: Dict[SwitchNode, List[CaseNode]] = dict()
# python decompile.py ../../Downloads/test_condition_for_eva test17 --debug
Copy link
Member

Choose a reason for hiding this comment

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

?

Copy link
Collaborator

Choose a reason for hiding this comment

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

removed

@steffenenders steffenenders merged commit 427a963 into main Jul 4, 2023
@steffenenders steffenenders deleted the issue-20-_Switch_Restructuring_Insert_CaseNodes branch July 4, 2023 08:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@steffenenders steffenenders steffenenders left review comments

@ebehner ebehner ebehner left review comments

Assignees

@ebehner ebehner

Labels
feature-request New feature or request priority-low Low priority issue
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Switch Restructuring] Insert CaseNodes
2 participants
@ebehner @steffenenders