23 lines
1.4 KiB
Text
23 lines
1.4 KiB
Text
|
Notes from code review of teamasdf
|
||
|
|
=========================================
|
||
|
|
|
||
|
|
To simplify this summary, the remarks said by other groups will be prefixed with "-", and the responses from teamasdf with "*"
|
||
|
|
|
||
|
|
Most problems have been noted by teamasdf in their presentation of their code.
|
||
|
|
|
||
|
|
- Usage of emitRaw instead of emit or emitMove (intermittent)
|
||
|
|
* The coding style changed along the project, and for ditions we forgot to use the proper methods
|
||
|
|
- There is an empty if somewhere
|
||
|
|
* Legacy from now removed print statements
|
||
|
|
- Unnecessary comments containing just code (such as the block comment in builtInRead)
|
||
|
|
* Again legacy code that should have been deleted
|
||
|
|
- Some minor things: unused variable in unaryOp and the missing @Override
|
||
|
|
- In the register counter visitor, the overriding of methods with the same method body as the super-method is not a good practice.
|
||
|
|
* Noted during the presentation
|
||
|
|
- The stack alignment in methodDecl that may have been necessary when a bug was present is not necessary once fixed
|
||
|
|
- Some labels are unused
|
||
|
|
- The emitRaw statements are copied from a compiled example program and is thus not as easy to read (but may be more efficient)
|
||
|
|
- The RegisterCountVisitor is located in cd.ir instead of cd.backend.codegen
|
||
|
|
- The RegisterCountVisitor<A> could have extended ExprVisitor<Integer, Void> instead of ExprVisitor<Integer, A> and having a type parameter.
|
||
|
|
|