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

Fix issue #111 on switch/case/default statements. Ensure that the swi… #112

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
9 changes: 5 additions & 4 deletions ppci/lang/c/parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -876,9 +876,10 @@ def parse_switch_statement(self):
self.consume("(")
expression = self.parse_expression()
self.consume(")")
self.semantics.on_switch_enter(expression)
# expression type can be changed by integer promotion
prom_expression = self.semantics.on_switch_enter(expression)
Copy link
Owner

Choose a reason for hiding this comment

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

I would simply rename this to expression again. I was confused that it was a promised expression.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No problem. In fact "prom_expression" was for "promoted_expression"
I used a new name to highlight the fact that the object it references can be different from the object referenced by "expression" (then I added the comment to clarify)

statement = self.parse_statement()
return self.semantics.on_switch_exit(expression, statement, location)
return self.semantics.on_switch_exit(prom_expression, statement, location)

def parse_case_statement(self):
""" Parse a case.
Expand All @@ -891,11 +892,11 @@ def parse_case_statement(self):
'case 5 ... 10:'
"""
location = self.consume("case").loc
value = self.parse_expression()
value = self.parse_constant_expression()
if self.peek == "...":
# gnu extension!
self.consume("...")
value2 = self.parse_expression()
value2 = self.parse_constant_expression()
value = (value, value2)

self.consume(":")
Expand Down
60 changes: 55 additions & 5 deletions ppci/lang/c/semantics.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,37 @@
from .printer import expr_to_str, type_to_str


class CSwitch:
""" Switch instruction context """
def __init__(self, typ):
self.typ = typ
self.default_seen = False
self.values = []
self.ranges = []

def add_value(self, val):
""" Add a case value, returns True if OK, False otherwise """
for low, high in self.ranges:
if val >= low and val <= high:
Copy link
Owner

Choose a reason for hiding this comment

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

Did you know that python has chained comparison logic?
In this case, you could use:

if low <= val <= high:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it's a neat feature. But I am careful with it because it pushes me to use it in other languages (eg C) where it is syntactically accepted but the result is not what is expected!

return False
for value in self.values:
if val == value:
return False
self.values.append(val)
return True

def add_range(self, val1, val2):
""" Add a case range, returns True if OK, False otherwise """
for low, high in self.ranges:
if (val1 >= low and val1 <= high) or (val2 >= low and val2 <= high):
return False
for value in self.values:
if value >= val1 and value <= val2:
return False
self.ranges.append((val1, val2))
return True


class CSemantics:
""" This class handles the C semantics """

Expand Down Expand Up @@ -540,7 +571,10 @@ def on_if(self, condition, then_statement, no, location):
return statements.If(condition, then_statement, no, location)

def on_switch_enter(self, expression):
self.switch_stack.append(expression.typ)
self.ensure_integer(expression)
prom_expression = self.promote(expression)
self.switch_stack.append(CSwitch(prom_expression.typ))
Copy link
Owner

Choose a reason for hiding this comment

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

This is a big improvement, to provide a switch context!

return prom_expression

def on_switch_exit(self, expression, statement, location):
""" Handle switch statement """
Expand All @@ -553,19 +587,35 @@ def on_case(self, value, statement, location):
self.error("Case statement outside of a switch!", location)

if isinstance(value, tuple):
# case value1 ... value2:
value1, value2 = value
value1 = self.coerce(value1, self.switch_stack[-1])
value2 = self.coerce(value2, self.switch_stack[-1])
self.ensure_integer(value1)
self.ensure_integer(value2)
value1 = self.promote(value1)
value2 = self.promote(value2)
v1 = self.context.eval_expr(value1)
v2 = self.context.eval_expr(value2)
if v1 > v2:
self.error("Inconsistent case range", location)
if not self.switch_stack[-1].add_range(v1, v2):
self.error("Case range conflicts with previous cases", location)
return statements.RangeCase(value1, value2, statement, location)
else:
value = self.coerce(value, self.switch_stack[-1])
# case value:
self.ensure_integer(value)
value = self.promote(value)
v = self.context.eval_expr(value)
if not self.switch_stack[-1].add_value(v):
self.error("Case value conflicts with previous cases", location)
return statements.Case(value, statement, location)

def on_default(self, statement, location):
""" Handle a default label """
if not self.switch_stack:
self.error("Default statement outside of a switch!", location)

if self.switch_stack[-1].default_seen:
self.error("Duplicate default statement in current switch", location)
self.switch_stack[-1].default_seen = True
return statements.Default(statement, location)

def on_while(self, condition, body, location):
Expand Down