Conversation
struct Type now has a property 'bool is_anytype' to represent the anytype. Anytype can be used to override the typechecker. For example, calling a memory allocation function returns a pointer, which can be a used as a pointer to any other type, whatever is put there. Anytype should have the highest native width of the architecture which is targeted, so 8 bytes for x86-64, and 2 bytes for AVR.
WalkthroughThis update introduces a new Boolean field ( Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant TC as tc_type_contains
Caller->>TC: Call tc_type_contains(expect, actual)
alt Either type has is_anytype=true
TC-->>Caller: Return true
else
TC->>TC: Perform pointer/basic/array type checks
TC-->>Caller: Return comparison result
end
sequenceDiagram
participant Infer as infer_type_expr
participant PA as infer_type_expr_ptr_arithmetic
Infer->>PA: Call with t1, t2, op
alt t2 is pointer and op is a comparison operator
PA-->>Infer: Return boolean type
else
PA->>PA: Execute pointer arithmetic logic
PA-->>Infer: Return computed type
end
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
✅ Deploy Preview for espl1000 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
examples/stdlib/base/allocator/loop_test/loop_test.dg (2)
1-20: Memory leak stress test looks good but has room for improvementThe test effectively verifies that the allocator can handle repeated allocations and deallocations without memory exhaustion.
Consider these improvements:
- Add a variable size allocation test to ensure the allocator handles different memory sizes
- Test memory access after allocation to verify memory integrity
- Consider adding output to report the test progress or final status
fn main () -> int { - // test that the allocations do not exhaus memory + // test that the allocations do not exhaust memory for i in 0 .. 10000 { *uint64 p = malloc(3000); if p == 0 { return 1; } + + // Verify memory is accessible + *uint8 bytePtr = p; + *bytePtr = 42; + if *bytePtr != 42 { + return 1; + } if free(p) != 0 { return 1; } } return 0; }
4-5: Fix typo in commentThere's a typo in the comment.
- // test that the allocations do not exhaus memory + // test that the allocations do not exhaust memoryexamples/stdlib/base/allocator/use_allocator/use_allocator.dg (1)
20-28: Ensure pointer arithmetic stays within boundsThe pointer arithmetic operations appear to stay within the allocated bounds, but this could be made more explicit with boundary checks or comments.
Consider adding boundary validation for safer pointer manipulation:
local *uint8 p; p = ptr; + // Memory boundary check + if ptr == 0 || 8349 < 110 { + free(ptr); + return 4; + } + p += 100; *p = 84; p += 10; *p = 92; p -= 10;examples/stdlib/base/allocator/no_overlap/no_overlap.dg (1)
7-13: Consider making the overlap logic into a reusable functionThe overlap detection logic is duplicated for both pointers. Consider extracting this logic into a separate function for better reusability and code maintenance.
- if ((ptr1 + 30) > ptr2 && (ptr1 <= ptr2)){ - return 1; - } - - if ((ptr2 + 30) > ptr1 && (ptr2 <= ptr1)){ - return 1; - } + fn do_overlap(*uint64 a, *uint64 b, uint64 size) -> bool { + return (a + size) > b && (a <= b); + } + + if (do_overlap(ptr1, ptr2, 30) || do_overlap(ptr2, ptr1, 30)) { + return 1; + }stdlib/base/allocator.dg (1)
5-32: Implementation ofmalloclooks reasonable but has potential issues.The function allocates memory in page-size chunks, which is a valid approach. However:
- Storing the allocation size at the start and using pointer arithmetic might cause alignment issues on some architectures.
- The function assumes 16 bytes offset is sufficient, which may not be correct if
uint64is not 8 bytes on all platforms.Consider adding a comment about the memory layout and potential alignment requirements:
// preserve number of bytes for calling munmap later *ptr = n; +// Return pointer offset by 16 bytes (8 bytes for size storage + 8 bytes padding) +// This assumes uint64 is 8 bytes and maintains alignment requirements return ptr + 16;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (25)
ast/ast/ast_types.h(1 hunks)ast/test/test_str_ast.c(1 hunks)ast/util/copy_ast.c(1 hunks)ast/util/equals_ast.c(1 hunks)ast/util/str_ast.c(4 hunks)compiler/main/typechecker/tc_ifstmt.c(1 hunks)compiler/main/typechecker/tc_retstmt.c(2 hunks)compiler/main/typechecker/type_contains/tc_pointertype_contains.c(1 hunks)compiler/main/typechecker/type_contains/tc_type_contains.c(1 hunks)compiler/main/typeinference/typeinfer_expr.c(4 hunks)docs/html/grammar.html(2 hunks)examples/stdlib/base/allocator/alloc_0_bytes/alloc_0_bytes.dg(1 hunks)examples/stdlib/base/allocator/alloc_0_bytes/alloc_0_bytes.exitcode(1 hunks)examples/stdlib/base/allocator/loop_test/loop_test.dg(1 hunks)examples/stdlib/base/allocator/loop_test/loop_test.exitcode(1 hunks)examples/stdlib/base/allocator/no_overlap/no_overlap.dg(1 hunks)examples/stdlib/base/allocator/no_overlap/no_overlap.exitcode(1 hunks)examples/stdlib/base/allocator/realloc_test/realloc_test.dg(1 hunks)examples/stdlib/base/allocator/realloc_test/realloc_test.exitcode(1 hunks)examples/stdlib/base/allocator/use_allocator/use_allocator.dg(1 hunks)examples/stdlib/base/allocator/use_allocator/use_allocator.exitcode(1 hunks)parser/main/astnodes/types/SimpleType.c(0 hunks)parser/main/astnodes/types/Type.c(2 hunks)parser/test/astnodes/types/SimpleTypeTest.c(2 hunks)stdlib/base/allocator.dg(1 hunks)
💤 Files with no reviewable changes (1)
- parser/main/astnodes/types/SimpleType.c
🧰 Additional context used
🧬 Code Definitions (4)
compiler/main/typechecker/type_contains/tc_pointertype_contains.c (2)
compiler/main/typechecker/type_contains/tc_type_contains.c (1)
tc_type_contains(10-47)compiler/main/typechecker/type_contains/tc_type_contains.h (1)
tc_type_contains(9-9)
compiler/main/typechecker/tc_ifstmt.c (1)
ast/util/str_ast.c (1)
str_type(286-302)
compiler/main/typechecker/tc_retstmt.c (1)
compiler/main/typechecker/type_contains/tc_type_contains.c (1)
tc_type_contains(10-47)
compiler/main/typeinference/typeinfer_expr.c (2)
compiler/main/typeinference/util/type_str.c (1)
typeFromStrPrimitive(75-83)compiler/main/typeinference/util/type_str.h (1)
typeFromStrPrimitive(5-5)
🔇 Additional comments (35)
examples/stdlib/base/allocator/use_allocator/use_allocator.exitcode (1)
1-2: Verify the Intended Exit Code.
The file defines an exit code of84. Please confirm that this code is intentional—typically, a non-zero exit code is used to indicate an error condition. Ensure that this value aligns with the expected behavior of the associateduse_allocator.dgtest.examples/stdlib/base/allocator/loop_test/loop_test.exitcode (1)
1-2: Approved Exit Code for Loop Test.
The exit code is set to0, indicating a successful test outcome. This is consistent with similar exit code files and appears appropriate for the loop test scenario.examples/stdlib/base/allocator/realloc_test/realloc_test.exitcode (1)
1-2: Validated Realloc Test Exit Code.
The file specifies an exit code of0, representing a successful state for the realloc test. This aligns with the expected behavior given the realloc scenarios in the corresponding test file.examples/stdlib/base/allocator/no_overlap/no_overlap.exitcode (1)
1-2: Approved Exit Code for No-Overlap Test.
The file provides an exit code of0, which is appropriate for a successful memory overlap check. Please ensure that the relatedno_overlap.dgtest anticipates this success code.examples/stdlib/base/allocator/alloc_0_bytes/alloc_0_bytes.exitcode (1)
1-2: Approved Exit Code for Allocating 0 Bytes.
The exit code is designated as0, indicating that the edge case of allocating 0 bytes is handled as a successful operation. This is acceptable if the corresponding test is designed to pass under these conditions.examples/stdlib/base/allocator/alloc_0_bytes/alloc_0_bytes.dg (1)
1-13: LGTM: Good test formalloc(0)behavior.This test properly checks for the implementation-defined behavior of
malloc(0), which according to C standard can either return NULL or a valid pointer (that shouldn't be dereferenced but can be passed tofree()).ast/ast/ast_types.h (1)
72-76: LGTM: Good addition ofis_anytypeflag with clear documentation.The new boolean field with its descriptive comment clearly explains the purpose - to represent a type that can stand for any other type, with architecture-specific width considerations.
ast/util/copy_ast.c (1)
182-182: LGTM: Properly copying the newis_anytypefield.The added line ensures that the
is_anytypeproperty is preserved when a type is duplicated, maintaining consistency with how other type properties are handled in the copy function.ast/test/test_str_ast.c (1)
107-107: LGTM: Updated test expectation for improved operator spacing.The test has been properly updated to match the new string representation format that includes spaces around operators, which improves code readability.
compiler/main/typechecker/type_contains/tc_type_contains.c (1)
12-14: Correct implementation ofanytypetype compatibilityThe added check properly implements the behavior where any type marked as
is_anytypeis considered compatible with any other type. This early return condition ensures that type checking is bypassed when dealing withanytype.examples/stdlib/base/allocator/loop_test/loop_test.dg (2)
14-16:Details
✅ Verification successful
Verify free return value expectations
The code checks if
free(p) != 0, assumingfreereturns a status code.
🏁 Script executed:
#!/bin/bash # Check how free is defined in the codebase rg "fn free" -A 3Length of output: 164
Free Return Behavior Validated
The
freefunction is declared with the signaturefn free(*# ptr) -> int(as seen instdlib/base/allocator.dg), confirming that it returns an integer. This supports the check inexamples/stdlib/base/allocator/loop_test/loop_test.dgwhere the returned value is compared against zero. Please ensure that the convention infreeis documented—that is, it returns0on success and a non-zero value on error. If that is already the case, no further changes are necessary.
8-8:Details
❓ Verification inconclusive
Verify if the pointer type is appropriate for storing malloc result
Storing the malloc result in a
*uint64pointer might not be the most appropriate type.
🏁 Script executed:
#!/bin/bash # Check how malloc is defined in the codebase rg "fn malloc" -A 3Length of output: 196
Action: Confirm Pointer Type Conversion for
mallocAllocationThe
mallocfunction (defined instdlib/base/allocator.dgasfn malloc(uint64 nbytes) -> *#) returns a generic pointer. In the code atexamples/stdlib/base/allocator/loop_test/loop_test.dg(line 8), the result ofmalloc(3000)is being stored in a*uint64variable. Please verify that:
- An implicit conversion from the generic pointer (
*#) to*uint64is permitted by the language's conversion rules, or whether an explicit cast is needed.- The allocated byte size (3000) is appropriate when allocating space for
uint64values (i.e. consider if the size should account for the size ofuint64elements).ast/util/equals_ast.c (1)
20-22: Appropriate equality check for anytypeThe implementation correctly handles the equality of
anytypetypes, ensuring that two types marked asanytypeare considered equal.Note that this differs from the containment logic in
tc_type_contains, where either type beinganytypeis sufficient for compatibility. This difference is appropriate since equality is stricter than containment.compiler/main/typechecker/tc_ifstmt.c (1)
38-43: Enhanced error reporting for type mismatch in if conditionsThe addition of these lines improves error diagnostics by showing the actual type found in an if statement condition when a boolean is expected. This integrates well with the new
is_anytypefeature, as thestr_typefunction now handles this flag correctly.compiler/main/typechecker/type_contains/tc_pointertype_contains.c (1)
25-25: Updated pointer type compatibility check to use containmentReplacing
eq_typewithtc_type_containsis a significant change that makes type checking more flexible. This change aligns with the newis_anytypefeature, as thetc_type_containsfunction now returns true if either type is marked asis_anytype, which allows pointers to be compatible with broader sets of types.examples/stdlib/base/allocator/no_overlap/no_overlap.dg (1)
2-19: Well-structured memory overlap detection exampleThis is a good example demonstrating basic memory allocation and overlap detection. The function correctly:
- Allocates two memory blocks
- Checks for potential overlap in both directions
- Properly frees the allocated memory
The code provides a clear test case for the allocator functionality.
parser/main/astnodes/types/Type.c (2)
76-76: Initialize is_anytype fieldProperly initializes the new
is_anytypefield tofalseby default, ensuring that types are not considered "any type" unless explicitly specified.
93-96: Added support for ANYTYPE tokenThis change properly handles the
ANYTYPEtoken by consuming it and setting theis_anytypeflag to true. The implementation follows the pattern established for other type tokens in this switch statement.examples/stdlib/base/allocator/realloc_test/realloc_test.dg (1)
2-29: Good allocator test implementation.The test case properly validates memory allocation and reallocation behavior:
- Checks initial allocation success
- Verifies that reallocating to a larger size returns a different pointer
- Confirms that reallocating to a size that can fit in the existing block reuses the same pointer
The error code approach provides clear diagnostics for identifying which assertion failed.
docs/html/grammar.html (2)
63-63: Grammar update to support anytype notation.The Type definition has been expanded to include the
#symbol as a valid type representation, which aligns with the introduction of theis_anytypefield in the Type struct.
81-81: Grammar update for StructType.This change correctly restricts StructType to only uppercase alphanumeric sequences, removing the
#symbol option that was moved to the Type definition.compiler/main/typechecker/tc_retstmt.c (2)
14-14: New include for type containment check.The inclusion of the header file for type containment checks is consistent with the change in return type checking logic.
35-35: Type checking logic enhancement.The change from direct equality (
eq_type) to containment checking (tc_type_contains) is a significant improvement that enables more flexible type checking, especially with the new anytype support. This allows a function to accept more specific types when an anytype is expected.parser/test/astnodes/types/SimpleTypeTest.c (2)
11-11: Include Type.h header for anytype support.Adding the Type.h header is necessary for accessing the new Type structure with the is_anytype field.
60-60: Update test to verify anytype parsing.The test case has been properly updated to:
- Use makeType2 instead of makeSimpleType
- Verify that the is_anytype flag is set correctly when parsing the # symbol
- Use the appropriate free_type function for cleanup
This ensures the anytype parsing functionality works as expected.
Also applies to: 64-64, 67-67
ast/util/str_ast.c (5)
296-298: Support foris_anytypeflag looks good.The added code properly handles the new
is_anytypefield in the Type struct, returning the string representation "#" when the flag is set.
501-501: Good adjustment to string length calculation.The length calculation now correctly accounts for the additional spaces between terms in the expression string representation.
509-509: Improved expression formatting with spaces.Adding spaces between terms and operators improves readability of generated expressions.
541-547: Good addition of relational operators.The implementation now supports string representation for all relational operators (==, !=, >=, >, <=, <), which is necessary for complete operator handling.
550-552: Improved error handling in the default case.Properly freeing memory and returning NULL in the default case prevents memory leaks when an unrecognized operator is encountered.
stdlib/base/allocator.dg (1)
35-49: Implementation offreelooks correct.The function correctly retrieves the allocation size and calls
munmapwith appropriate parameters. Good check for null pointer.compiler/main/typeinference/typeinfer_expr.c (4)
24-24: Good addition of operator parameter to function signature.Adding the
opparameter toinfer_type_expr_ptr_arithmeticallows for more context-aware type inference.
47-48: Updated function calls correctly pass the operation parameter.The calls to
infer_type_expr_ptr_arithmeticnow correctly pass theopparameter, maintaining consistency with the updated function signature.Also applies to: 50-51
94-98: Good addition of pointer comparison type inference.Added logic correctly handles comparisons between two pointer types, returning a boolean type for all comparison operators.
101-108: Improved variable naming and added boolean return type for comparisons.Renaming from
sttostypeimproves code clarity, preventing confusion with the symbol table parameter. The added logic for comparison operators correctly returns a boolean type.
It allocates in multiples of systems page size. It does not do any internal bookkeeping at all. To summarize, it is very inefficient. But it should suffice for creating simple programs which need a heap.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
examples/stdlib/base/allocator/realloc_test/realloc_test.dg (1)
1-35: Consider adding memory deallocation.While not strictly necessary for a test program that immediately exits, it's good practice to free allocated memory with
free(p3)before returning.return 0; + free(p3); + return 0; }This would demonstrate complete memory management and prevent static analyzers from flagging memory leaks.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
compiler/main/typeinference/typeinfer_expr.c(4 hunks)examples/stdlib/base/allocator/alloc_0_bytes/alloc_0_bytes.dg(1 hunks)examples/stdlib/base/allocator/alloc_0_bytes/alloc_0_bytes.exitcode(1 hunks)examples/stdlib/base/allocator/loop_test/loop_test.dg(1 hunks)examples/stdlib/base/allocator/loop_test/loop_test.exitcode(1 hunks)examples/stdlib/base/allocator/no_overlap/no_overlap.dg(1 hunks)examples/stdlib/base/allocator/no_overlap/no_overlap.exitcode(1 hunks)examples/stdlib/base/allocator/realloc_test/realloc_test.dg(1 hunks)examples/stdlib/base/allocator/realloc_test/realloc_test.exitcode(1 hunks)examples/stdlib/base/allocator/use_allocator/use_allocator.dg(1 hunks)examples/stdlib/base/allocator/use_allocator/use_allocator.exitcode(1 hunks)stdlib/base/allocator.dg(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (11)
- examples/stdlib/base/allocator/alloc_0_bytes/alloc_0_bytes.exitcode
- examples/stdlib/base/allocator/no_overlap/no_overlap.exitcode
- examples/stdlib/base/allocator/loop_test/loop_test.exitcode
- examples/stdlib/base/allocator/use_allocator/use_allocator.exitcode
- examples/stdlib/base/allocator/realloc_test/realloc_test.exitcode
- examples/stdlib/base/allocator/alloc_0_bytes/alloc_0_bytes.dg
- examples/stdlib/base/allocator/loop_test/loop_test.dg
- examples/stdlib/base/allocator/no_overlap/no_overlap.dg
- examples/stdlib/base/allocator/use_allocator/use_allocator.dg
- compiler/main/typeinference/typeinfer_expr.c
- stdlib/base/allocator.dg
🔇 Additional comments (5)
examples/stdlib/base/allocator/realloc_test/realloc_test.dg (5)
2-35: Well-structured realloc test implementation.This test effectively validates the basic behavior of the memory allocation and reallocation functions, with proper error handling and clear assumptions about expected memory management behavior.
4-8: Good initial allocation and error handling.The code correctly tests the initial memory allocation and handles the failure case appropriately by returning an error code.
10-20: Proper verification of reallocated memory content.The test correctly verifies that the reallocated memory preserves the contents of the original allocation, which is a critical requirement for
realloc.
22-25: Consider implementation-specific behavior.The assumption that the pointer must change when increasing allocation size significantly is reasonable, but be aware that some malloc implementations might be able to expand in-place if there's adjacent free space.
However, this is a good test case for most common implementations.
27-32: Similar implementation-specific consideration.The assumption that the pointer should remain the same when reducing allocation size is reasonable, but some implementations might still relocate the memory.
This test will help identify those implementations that perform unnecessary relocations.
1 participant
Summary by CodeRabbit
New Features
Documentation
Tests