Skip to content
Draft
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
88 changes: 68 additions & 20 deletions src/cfile/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ class DataType(Element):
"""
Base class for all data types
"""
def __init__(self, name: str | None) -> None:
def __init__(self, name: str = "") -> None:

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Why change this? It's off topic

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Less complicated API. Because what is difference between "" or None? You are checking for None but not for "". So it is simpler and easier to check for "" then for None and ""

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

So based on what you said bellow i understand why you are using None, but still i think it is redundant.

self.name = name


Expand All @@ -150,7 +150,7 @@ def __init__(self,
pointer: bool = False,
volatile: bool = False,
array: int | None = None) -> None: # Only used for typedefs to other array types
super().__init__(None)
super().__init__("")
self.base_type = base_type
self.const = const
self.volatile = volatile
Expand All @@ -168,6 +168,45 @@ def qualifier(self, name) -> bool:
else:
raise KeyError(name)

class EnumMember(Element):

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This should be named EnumValue

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Actually not. It is like StructMember. This is also member of Enum type, which has name and value

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Another point of view:
If this should be EnumValue, then StructMember should be StructVariable.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

How about StructElement?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Well it is up to you. But it should be usable for both cases. I am coauthor of eRPC project (similar to this) and at the beginning i was lead by USA architect who decided to use StructMember and EnumMember. Sounds good to me.

https://github.com/EmbeddedRPC/erpc/tree/develop/erpcgen/src/types

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Could be also StructItem and EnumItem

"""
Enum element.
"""

def __init__(self,
name: str,
value: int | None) -> None:
self.name = name
self.value = value


class Enum(DataType):
"""
A enum definition
"""

def __init__(self, name: str, members: list[EnumMember] = [], attributes: list[str] = []) -> None:

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

You must never use empty list as default value in functions. It's dangerous.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Why? It is valid case, where you have no members.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

t's a very common mistake among newcomers to Python. Google it, there are great explanations on the internet.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

That just sad.
Ok we can use None. But i wouldn't use one membert type as an option.
so list[EnumMember] | None = None
in a code bellow:
self.members = list(working_list) if working_list else []

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

It's safer to do a proper check against None.

def __init__(self, name: str, elements:  list[EnumElement] | None = None):
self.elements: list[EnumElement] = []

if elements is not None:
    self.elements = list(elements)

These days however I always take it one step further and double-check that each individual element in the given argument list is of expected type. I usually do this by calling a helper function (usually called append or append_<something> that does the actual check.

super().__init__(name)
if isinstance(members, list):
self.members: list[EnumMember] = members.copy()
enumValue = -1
for enum in self.members:
if enum.value == None:
enumValue += 1
enum.value = enumValue
else:
enumValue = enum.value
else:
raise TypeError('Invalid argument type for "elements"')
self.attributes = attributes.copy()

def append(self, member: EnumMember) -> None:
"""
Appends new element to the struct definition
"""
if not isinstance(member, EnumMember):
raise TypeError(f'Invalid type, expected "EnumMember", got {str(type(member))}')
self.members.append(member)

class StructMember(Element):
"""
Expand Down Expand Up @@ -197,17 +236,13 @@ class Struct(DataType):
"""
A struct definition
"""
def __init__(self, name: str | None, members: StructMember | list[StructMember] | None = None) -> None:
def __init__(self, name: str = "", members: list[StructMember] = [], attributes: list[str] = []) -> None:

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Never use empty list as initializer.
Introducing attributes here needs some further design considerations. Attributes are compiler-specific for GCC. What if we are using MSVC? We should introduce the attribute mechanism separately.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I agree, i just need this one so i implemented them this way. But i was thinking to have something like cfiles attribute class

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I agree, i just need this one so i implemented them this way. But i was thinking to have something like cfiles attribute class

super().__init__(name)
self.members: list[StructMember] = []
if members is not None:
if isinstance(members, StructMember):
self.append(members)
elif isinstance(members, list):
for member in members:
self.append(member)
else:
raise TypeError('Invalid argument type for "elements"')
if isinstance(members, list):
self.members: list[StructMember] = members.copy()
else:
raise TypeError('Invalid argument type for "elements"')
self.attributes = attributes.copy()

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Don't use copy. If you want to create a new list which is a copy of another, write it as list(attributes).
But as stated above, we should introduce attribute-mechanism later anyway.


def append(self, member: StructMember) -> None:
"""
Expand Down Expand Up @@ -322,7 +357,7 @@ def __init__(self,
static: bool = False,
const: bool = False, # const function (as seen in C++)
extern: bool = False,
params: Variable | list[Variable] | None = None) -> None:
params: list[Variable] = []) -> None:

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Empty list as default value

self.name = name
self.static = static
self.const = const
Expand All @@ -335,13 +370,7 @@ def __init__(self,
self.return_type = Type("void")
else:
raise TypeError(str(type(return_type)))
self.params: list[Variable] = []
if params is not None:
if isinstance(params, Variable):
self.append(params)
elif isinstance(params, list):
for param in params:
self.append(param)
self.params = params.copy()

def append(self, param: Variable) -> "Function":
"""
Expand Down Expand Up @@ -511,3 +540,22 @@ class Block(Sequence):
"""
A sequence wrapped in braces
"""


class ConditionType(Enum):
IF = 0
ELSE_IF = 1
ELSE = 2


class Condition(Block):

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Now you're taking shortcuts. Not all if-statements are wrapped in braces.
In C you can declare statements like this:

if(condition) ++x; else ++y;

"""
A condition wrapped in braces
"""
def __init__(self, condition: str = "", type: ConditionType = ConditionType.ELSE):
super().__init__()
self.condition = condition
self.type = type

if (condition and type == ConditionType.ELSE) or (not condition and type != ConditionType.ELSE):
raise Exception("Condition and type didn't match")
35 changes: 30 additions & 5 deletions src/cfile/factory.py
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ def function(self,
static: bool = False,
const: bool = False, # This is not const of the return type
extern: bool = False,
params: core.Variable | list[core.Variable] | None = None) -> core.Function:
params: list[core.Variable] = []) -> core.Function:
"""
New function
"""
Expand All @@ -137,6 +137,23 @@ def type(self,
"""
return core.Type(type_ref, const, pointer, volatile, array)

def enum_member(self,
name: str,
value: int | None = None):
"""
New EnumMember
"""
return core.EnumMember(name, value)

def enum(self,
name: str,
members: list[core.EnumMember] = [],
attributes: list[str] = []):
"""
New Enum
"""
return core.Enum(name, members, attributes)

def struct_member(self,
name: str,
data_type: str | core.Type | core.Struct,
Expand All @@ -150,12 +167,13 @@ def struct_member(self,

def struct(self,
name: str,
members: core.StructMember | list[core.StructMember] | None = None
members: list[core.StructMember] = [],
attributes: list[str] = []
) -> core.Struct:
"""
New Struct
"""
return core.Struct(name, members)
return core.Struct(name, members, attributes)

def variable(self,
name: str,
Expand Down Expand Up @@ -224,6 +242,13 @@ def func_return(self, expression: int | float | str | core.Element) -> core.Func
def declaration(self,
element: Union[core.Variable, core.Function, core.DataType],
init_value: Any | None = None) -> core.Declaration:

"""New declaration"""
"""
New declaration
"""
return core.Declaration(element, init_value)

def condition(self, condition:str, type:core.ConditionType):
"""
New condition
"""
return core.Condition(condition, type)
Loading