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

24b Improvements: Decoupling #64

Merged
merged 33 commits into from
May 22, 2022
Merged

24b Improvements: Decoupling #64

merged 33 commits into from
May 22, 2022

Conversation

ngjunsiang
Copy link
Contributor

pseudocode/interpreter.py:323: error: "Stmt" has no attribute "rule"

Let's start with this:

class Stmt:
rule: str = NotImplemented

This tells Python that the Stmt class has a rule class attribute, and any class that inherits this should override the rule attribute (or risk getting a NotImplemented value when trying to access it`.

And now that message goes away when we run mypy again.

@ngjunsiang
Copy link
Contributor Author

We should define interfaces like this for any class that gets inherited. Let's look at Expr:

class Expr:
"""
Represents an expression in 9608 pseudocode.
An expression can be resolved to a Type,
and evaluated to a Value.
An Expr must return an associated token for error-reporting purposes.
Methods
-------
- token() -> Token
Returns the token asociated with the expr
"""
def __repr__(self) -> str:
attrstr = ", ".join([
repr(getattr(self, attr)) for attr in self.__slots__
])
return f'{type(self).__name__}({attrstr})'
def token(self) -> "Token":
raise NotImplementedError

Not having an __init__() makes it easier to implement the subclasses flexibly, while still ensuring that they implement a way to retrieve a token.

Immediate implications: looks like we'll have to decouple Literal from TypedValue and give it its own __init__():

class Literal(Expr):
"""
A Literal represents any value coming directly from
the source code.
"""
def __init__(self, type: Type, value: Val, *, token: "Token") -> None:
self.type = type
self.value = value
self._token = token
def token(self) -> "Token":
return self._token

And it now implements its own token() method as well. We'll do the same for each Expr subclass; tedious, but has some advantages as we'll see later.

@ngjunsiang
Copy link
Contributor Author

Looking at our expectations of what an Expr is and does:

class Expr:
"""
Represents an expression in 9608 pseudocode.
An expression can be resolved to a Type,
and evaluated to a Value.
An Expr must return an associated token for error-reporting purposes.
Methods
-------
- token() -> Token
Returns the token asociated with the expr
"""

It seems like Name should no longer be considered an Expr: it does not resolve to a Type, and doesn't evaluate to a Value as well (note that names are not one of pseudo's types). So we take out its inheritance from Expr:

class Name:
__slots__ = ('name',)
def __init__(
self,
name: Varname,
token: "Token",
) -> None:
self.name = name
self._token = token
def token(self) -> "Token":
return self._token

It would, however, still be useful to retain the token associated with the name, so we'll keep that.

@ngjunsiang
Copy link
Contributor Author

ngjunsiang commented May 22, 2022

The makeExpr() helper was useful while we were still unsure how tokens for error reporting were going to work; we could decide at the point of instantiation which token to use.

Now, many chapters later, this did not come to pass; for the most part each Expr will use a specific part of itself as the associated token. For example, Declare Exprs are always associated with their name token:

return makeExpr(
name=name.name,
type=typetoken.word,
metadata=metadata,
token=name.token(),
)

It no longer makes sense to be passing tokens to Expr at init time then. Let's update Declare: [425544d], [a678a47]

class Declare(Expr):
__slots__ = ('name', 'type', 'metadata')
def __init__(
self,
name: Name,
type: Type,
metadata: Mapping=None,
) -> None:
self.name = name.name
self.type = type
self.metadata = metadata
self._token = name
def token(self):
return self._token

Note that we no longer need to call super().__init__() since Expr has no init.

@ngjunsiang
Copy link
Contributor Author

ngjunsiang commented May 22, 2022

Similar changes for Unary and Binary: [7eec403], [5983f87]

They also lose the superclass init. Unfortunately, their operators are passed as functions: we generally want to avoid Exprs knowing about how tokens work; we can pass tokens around but try not to extract information from them manually.

This means we will have to instantiate Unary and Binary with a token= keyword argument.

@ngjunsiang
Copy link
Contributor Author

And finally, our most complex Exprs. Call no longer needs to hold on to a token: [9682e6e]

class Get(Expr):
__slots__ = ('frame', 'name', '_token')
def __init__(
self,
frame: "Frame",
name: Name,
) -> None:
self.frame = frame
self.name = name.name
self._token = name.token()
def token(self):
return self._token

class Call(Expr):
__slots__ = ('callable', 'args')
def __init__(
self,
callable: "Callable",
args: Iterable[Arg],
) -> None:
self.callable = callable
self.args = args
def token(self):
return self.callable.token()

@ngjunsiang
Copy link
Contributor Author

A few fixes in the way we use Name: [06b3e87]

This is mighty confusing. I'm going to redo this part when I figure out a way to pass names and tokens around more cleanly.

makeExpr() makes static type checking difficult. So let's work towards eliminating it, by replacing all makeExpr() calls with their appropriate class instantiations: [904a050]

And then remove it: [622be6d]

@ngjunsiang
Copy link
Contributor Author

More fixes:

  • I misunderstood the use of NewType; Arg is unnecessary and Index is just typed as a tuple of Exprs or tuple of ints: [ae5fd65]
  • Grouping the ExprStmts together for more consistent typing: [4793f4a]
  • value() is proving tricky to type properly; this is a huge function with many possible return types. I'll revisit this in a later refactoring chapter: [e96f1a9]

@ngjunsiang
Copy link
Contributor Author

Next, typing fixes for various statement types:

@ngjunsiang
Copy link
Contributor Author

Then some renaming for better readability: [98383cd]

Instead of Value, we now use PseudoValue to refer to pseudo's special value types (Callable, File, Array, Object, ...)

Value, on the other hand, refers to any value which can be represented in a TypedValue. So this includes PsuedoValues, bool, int, float, str.

@ngjunsiang ngjunsiang changed the title 24b Improvements: Typing hierarchy 24b Improvements: Decoupling May 22, 2022
@ngjunsiang
Copy link
Contributor Author

What follows is a lot of mole-whacking for typing inconsistencies, which are ... too numerous to detail here. I simply aimed to have working code for now, and next chapter we will go about organising things in a clearer fashion that will make it easier for us to reason about the code.

On the plate are:

  • clear parse/resolve/execute dispatch and return types
  • clearer object hierarchy

@ngjunsiang ngjunsiang merged commit 44001cf into main May 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant