implement parsing free conversion operators #95
Conversation
| @@ -697,7 +698,7 @@ def _parse_template_specialization(self) -> TemplateSpecialization: | |||
|
|
|||
| try: | |||
| parsed_type, mods = self._parse_type(None) | |||
| if parsed_type is None: | |||
There was a problem hiding this comment.
_parse_type now return Union of types. According to typing rules, there should be if checking which type is returned (that is the reason why function should not return Union)
| @@ -2331,8 +2332,6 @@ def _parse_type( | |||
| tok = get_token() | |||
|
|
|||
| pqname: typing.Optional[PQName] = None | |||
| pqname_optional = False | |||
| mods = ParsedTypeModifiers(vars, both, meths) | ||
| po = self._parse_member_operator() | ||
| return po, mods |
There was a problem hiding this comment.
Here we are parsing the old cases + some additional parsing that was in _parse_operator_conversion. I extracted that to a new function to make code a bit simpler.
cxxheaderparser/parser.py
Outdated
| po = self._parse_member_operator() | ||
| return po, mods | ||
|
|
||
| pqname, op = self._parse_pqname( |
There was a problem hiding this comment.
We must run _parse_pqname because we do not know if this is operator or another function. But if we already ran this function, we cannot return (or I do not found such functions) processed tokens,
There was a problem hiding this comment.
Also, I changed argument fn_ok to True to not raise error when operator is detected.
|
|
||
| if op is not None: | ||
| # special case: conversion operator, but also a free operator | ||
| mods = ParsedTypeModifiers(vars, both, meths) |
There was a problem hiding this comment.
This is only parsed when we detected operator as a free operator.
| ) -> Operator: | ||
| """This function parses operator implemented outside class body.""" | ||
| last_seg = pqname.segments[-1] | ||
| assert last_seg.name.startswith("operator") |
There was a problem hiding this comment.
This is a bit strange to me: why there is not used more complex type in NameSpecifier and operator name is concatenated? Anyway, we do not care here about this name. We take the real operator name from op variable.
There was a problem hiding this comment.
The operator name gets concatenated in _parse_pqname_name... I think it would be better for that function to not concatenate and let the eventual user of the operator make that call.
There was a problem hiding this comment.
ok, I changed _parse_pqname_name to not concatenate this. This caused even more large change for something that seems like it should be rather simple, because I changed return types for several functions from str|None to list[LexToken].
cxxheaderparser/parser.py
Outdated
| assert last_seg.name.startswith("operator") | ||
| last_seg.name = "operator" | ||
|
|
||
| type_name = PQName([NameSpecifier(p) for p in op.split(PlyLexer.t_DBL_COLON)]) |
There was a problem hiding this comment.
This is the part where I am the most hesitant, because we split here operator by ::.
It seems good enough (the code is parsed after all), but if there is a better way, I think it should be changed.
| tok = self._next_token_must_be("operator") | ||
|
|
There was a problem hiding this comment.
The token tok has already been processed in _parse_type.
There was a problem hiding this comment.
Actually, perhaps a more appropriate name for this function would be _parse_type_or_operator ?
cxxheaderparser/types.py
Outdated
| @dataclass | ||
| class Operator: | ||
| """An internal structure for parsing operator.""" | ||
|
|
||
| pqname: PQName | ||
| """Possibly qualified name for operator.""" | ||
| operator_name: str | ||
| """Conversion operator have always `conversion` str in this attribute.""" | ||
| ctype: Type | ||
| """Return type for this operator.""" | ||
| cmods: "ParsedTypeModifiers" | ||
| """Return type modifiers for this operator.""" | ||
|
|
There was a problem hiding this comment.
This is only for internal usage to store data between _parse_type and _parse_operator_conversion in _parse_declarations method.
(to be consistent with project conventions)
There was a problem hiding this comment.
This is a surprisingly large change for something that seems like it should be rather simple. However, I poked at it a little bit and I can see why it's complex, since this sort of operator violates some of my assumptions.
I might have time this weekend to try to find a simpler option.
cxxheaderparser/types.py
Outdated
| @@ -298,6 +301,23 @@ def format_decl(self, name: str): | |||
| return f"{c}{v}{self.typename.format()} {name}" | |||
|
|
|||
|
|
|||
| @dataclass | |||
| class Operator: | |||
There was a problem hiding this comment.
I think if this isn't exposed to users, it probably doesn't belong in types.py
There was a problem hiding this comment.
ok,
I had thought about it before, but since this module contains a lot of data classes, I decided to put it there.
I removed it from types.py and moved to parser.py, because only there it is used (I personally prefer to keep many small files, but I see that convention in this project is to keep small number of modules).
cxxheaderparser/parser.py
Outdated
| assert last_seg.name.startswith("operator") | ||
| last_seg.name = "operator" | ||
|
|
||
| type_name = PQName([NameSpecifier(p) for p in op.split(PlyLexer.t_DBL_COLON)]) |
| ) -> Operator: | ||
| """This function parses operator implemented outside class body.""" | ||
| last_seg = pqname.segments[-1] | ||
| assert last_seg.name.startswith("operator") |
There was a problem hiding this comment.
The operator name gets concatenated in _parse_pqname_name... I think it would be better for that function to not concatenate and let the eventual user of the operator make that call.
Closes #94