This is a LIMITED REVIEW: a time-bound effort to provide security insights on a codebase without reviewing it fully.
The subjects covered by our review are detailed in the Review Overview section.
We did not find any issues in the fixes of the security advisories that were in the scope of this review and
can confidently assert that the security advisories have been resolved.
The elimination of the Function Signature class enhances the code’s readability and consistency, according to our findings. This removal, enabled by the previous pull request that refactored the type system and the code generation, is one of the last steps in merging the type systems of the semantic analysis and the code generation.
The Journal and its commit/rollback scheme fix the issue with incorrect type checking of loop variables but also allows for future new metadata to be added to the compiler easily. Although one issue was found in its implementation as highlighted by Metadata Journal can rollback incorrectly, this new primitive is a great addition to the compiler as it also fixes a performance issue by caching the list of potential types for nodes.
Special attention should be applied to testing complex expressions with functions calls as sub-expression. As highlighted in various issues such as Multiple evaluations of DST lead to non-unique symbol errors when copying Bytes arrays or DynArrays or Make_setter is incorrect for complex types when the RHS references the LHS with a function call, such complex expressions might be edge cases in the compiler logic and should be part of the testing suite.
Additionally, the large amount of issues related to the new IfExp AST node depicts the importance for the compiler to be more generic in its way to validate the semantics of expressions as currently, some functions must handle the case of several AST nodes in distinct ways as they cannot be handled by the general logic.