octal notation, -Wall -Wextra -Werror -march=native#165
Conversation
probably leftovers when it was still transpiling to C code.
I had thought these were already enabled but apparently they were not. Lots of fixes to repair all the mistakes found by these warnings. The correct way to add these options in cmake seems to be the 'add_compile_options' directive. That should also apply then to any subdirecties below where it is set. [1] https://cmake.org/cmake/help/latest/command/add_compile_options.html#command
WalkthroughThis set of changes primarily focuses on code cleanup, modernization, and minor feature enhancements across the codebase. Major changes include removing unused variables, parameters, functions, and labels, as well as updating loop index types from Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Lexer
participant Parser
participant AST
User->>Lexer: Provide source code with integer literals (including octal)
Lexer->>Lexer: Detect integer literals
alt Starts with "0"
Lexer->>Lexer: Parse as octal literal
Lexer->>Parser: Output decimal value token
else
Lexer->>Parser: Output decimal/hex token as normal
end
Parser->>AST: Build AST (no includes handled)
AST->>User: AST without include declarations
Possibly related PRs
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (36)
✅ Files skipped from review due to trivial changes (21)
🚧 Files skipped from review as they are similar to previous changes (12)
🧰 Additional context used🧬 Code Graph Analysis (1)lexer/src/lexer.c (2)
🔇 Additional comments (11)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 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: 20
🔭 Outside diff range comments (3)
examples/other/everything.dg (1)
20-20:⚠️ Potential issueFix the return type arrow syntax.
The
mainfunction signature uses~>but the DSL elsewhere (e.g.,sub1,sub2) uses->for return types. Update to match the expected syntax:-fn main () ~> int { +fn main () -> int {tables/stst/stst.c (1)
158-159:⚠️ Potential issueFix implicit declaration of asprintf function
There's a pipeline failure due to the implicit declaration of the
asprintffunction. Although this isn't part of your changes, it's causing the build to fail.Add the following include at the top of the file to fix this issue:
#include <stdio.h> #include <stdlib.h> #include <string.h> #include <assert.h> + +// For asprintf function +#define _GNU_SOURCE +#include <stdio.h>Alternatively, you could replace the asprintf calls with more portable code:
-asprintf(&(line->_namespace), "%s", _namespace); -asprintf(&(line->name), "%s", s->type->struct_type->type_name); +line->_namespace = strdup(_namespace); +line->name = strdup(s->type->struct_type->type_name);🧰 Tools
🪛 CircleCI: build-job-ubuntu-2004
[error] 158-158: Implicit declaration of function 'asprintf'; did you mean 'vsprintf'? (error due to -Werror=implicit-function-declaration)
🪛 CircleCI: valgrind
[error] 158-158: implicit declaration of function ‘asprintf’; did you mean ‘vsprintf’? (gcc -Werror=implicit-function-declaration)
tables/lvst/lvst.c (1)
169-171: 🛠️ Refactor suggestionUpdate
printfformat specifiers after switching tosize_tLoop indices & counters were migrated to
size_t, but the related
printfstatements still use%d/%ld.
With-Wall -Wextra -Werrorthis will raise
-Wformaterrors on some platforms. Use the portable C99 specifier%zu.- fprintf(stderr, "lvst has %ld local vars, %ld args\n", nvars, nargs); + fprintf(stderr, "lvst has %zu local vars, %zu args\n", nvars, nargs); - printf(fmt, "name", "is_arg", "Type"); + printf(fmt, "name", "is_arg", "Type"); - for (size_t i = 0; i < lvst->count; i++) { + for (size_t i = 0; i < lvst->count; i++) { ... - printf("[LVST][Error] index %d not found in LVST\n", index); + printf("[LVST][Error] index %u not found in LVST\n", index);(same change applies to other
%ld/%dprintingsize_toruint32_t
values in this file.)Also applies to: 200-207, 330-338
🧹 Nitpick comments (2)
examples/other/everything.dg (1)
1-1: Complete or refine the example description comment.The opening comment
//The goal of this example is toends abruptly and leaves the reader guessing. Consider finishing the sentence to clearly communicate the example’s purpose. For instance:-//The goal of this example is to +// The goal of this example is to showcase all statement types and Smalldragon features for comprehensive testingrat/rat.c (1)
374-374: Added default return value forrat_base_ptrThe addition of a default return value for this function ensures it always returns something valid, even in unexpected cases. However, returning
0instead ofSD_REGISTER_ENDis inconsistent with the pattern used in other similar functions.Consider using
SD_REGISTER_ENDas the default return value for consistency with the other architecture-specific register functions:- return 0; + return SD_REGISTER_END;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (99)
CMakeLists.txt(1 hunks)ast/CMakeLists.txt(0 hunks)ast/ast.h(1 hunks)ast/test/CMakeLists.txt(0 hunks)ast/util/free_ast.c(1 hunks)ast/util/str_ast.c(2 hunks)ast/visitor/visitor.c(1 hunks)compiler/main/CMakeLists.txt(0 hunks)compiler/main/analyzer/dead/dead_analyzer.c(1 hunks)compiler/main/avr_code_gen/cg_avr_basic_block.c(0 hunks)compiler/main/avr_code_gen/compile_ir/compile_tac_binary_op.c(0 hunks)compiler/main/avr_code_gen/compile_ir/compile_tac_const_value.c(1 hunks)compiler/main/avr_code_gen/compile_ir/compile_tac_label.c(1 hunks)compiler/main/basic_block/basicblock.c(2 hunks)compiler/main/cli/main.c(1 hunks)compiler/main/compiler.c(3 hunks)compiler/main/gen_tac/gen_tac_assignstmt.c(0 hunks)compiler/main/gen_tac/gen_tac_call.c(1 hunks)compiler/main/gen_tac/gen_tac_expr.c(0 hunks)compiler/main/gen_tac/gen_tac_forstmt.c(0 hunks)compiler/main/gen_tac/gen_tac_term.c(0 hunks)compiler/main/liveness/liveness.c(6 hunks)compiler/main/typechecker/type_contains/tc_primitivetype_contains.c(2 hunks)compiler/main/typeinference/infer_in_context.c(0 hunks)compiler/main/x86_code_gen/allocate_registers_x86.c(3 hunks)compiler/main/x86_code_gen/compile_ir/compile_tac_binary_op.c(0 hunks)compiler/main/x86_code_gen/compile_ir/compile_tac_call.c(1 hunks)compiler/main/x86_code_gen/compile_ir/compile_tac_icall.c(1 hunks)compiler/main/x86_code_gen/compile_ir/compile_tac_label.c(1 hunks)compiler/main/x86_code_gen/compile_ir/compile_tac_load_local_addr.c(0 hunks)compiler/main/x86_code_gen/compile_ir/compile_tac_param.c(1 hunks)compiler/main/x86_code_gen/compile_ir/compile_tac_unary_op.c(0 hunks)compiler/test/CMakeLists.txt(0 hunks)compiler/test/avr_code_gen/compile_ir/test_compile_tac_store.c(1 hunks)compiler/test/avr_code_gen/timer/test_avr_code_gen_timer.c(0 hunks)compiler/test/gen_tac/test_gen_tac_simplevar.c(1 hunks)compiler/test/gen_tac/util/test_gen_tac_util.c(0 hunks)compiler/test/libvmcu_utils/libvmcu_utils.c(0 hunks)compiler/test/testcases.c(0 hunks)compiler/test/x86_code_gen/compile_ir/test_compile_tac_binary_op.c(4 hunks)compiler/test/x86_code_gen/compile_ir/test_compile_tac_if_cmp_goto.c(1 hunks)compiler/test/x86_code_gen/compile_ir/test_compile_tac_if_goto.c(2 hunks)compiler/test/x86_code_gen/compile_ir/test_compile_tac_return.c(1 hunks)compiler/test/x86_code_gen/compile_ir/test_compile_tac_unary_op.c(1 hunks)compiler/test/x86_code_gen/fake_lvst.c(1 hunks)compiler/test/x86_code_gen/test_x86_code_gen_util.c(6 hunks)examples/octal/octal_notation.dg(1 hunks)examples/octal/octal_notation.exitcode(1 hunks)examples/other/everything.dg(1 hunks)examples/stdlib/syscalls/write_file/write_file.dg(1 hunks)examples/tc_err.dg(0 hunks)examples/typeinference/localvartypeinference.dg(0 hunks)ibuffer/CMakeLists.txt(0 hunks)ibuffer/ibuffer_write.c(2 hunks)lexer/src/CMakeLists.txt(0 hunks)lexer/src/lexer.c(8 hunks)lexer/src/lexer_main.c(0 hunks)lexer/test/CMakeLists.txt(0 hunks)lexer/test/test.h(0 hunks)lexer/test/testcases/tests_include_decl.c(0 hunks)lexer/test/testcases/tests_other.c(1 hunks)parser/main/CMakeLists.txt(0 hunks)parser/main/astnodes/Namespace.c(2 hunks)parser/main/astnodes/expr/Term.c(1 hunks)parser/main/astnodes/statements/AssignStmt.c(3 hunks)parser/main/astnodes/types/Type.c(0 hunks)parser/main/astnodes/var/SimpleVar.c(1 hunks)parser/main/util/parser.c(0 hunks)parser/test/CMakeLists.txt(0 hunks)rat/CMakeLists.txt(0 hunks)rat/rat.c(6 hunks)rat/rat_x86.c(1 hunks)rat/rat_x86.h(1 hunks)rat/test/CMakeLists.txt(0 hunks)stdlib/base/math.dg(0 hunks)stdlib/collections/array.dg(0 hunks)stdlib/collections/linkedlist.dg(0 hunks)stdlib/collections/set.dg(0 hunks)stdlib/collections/stack.dg(0 hunks)tables/CMakeLists.txt(0 hunks)tables/lvst/lvst.c(21 hunks)tables/sst/sst.c(6 hunks)tables/sst/sst_fill.c(1 hunks)tables/sst/sst_print.c(1 hunks)tables/stst/stst.c(3 hunks)tables/stst/stst_print.c(1 hunks)tables/symtable/symtable.c(1 hunks)tables/test/CMakeLists.txt(0 hunks)tables/test/test.c(0 hunks)tac/CMakeLists.txt(0 hunks)tac/tacbuffer.c(2 hunks)tac/tacbuffer.h(1 hunks)tac/tacbuffer_optimize.c(1 hunks)tac/test/test_tac.c(0 hunks)token/CMakeLists.txt(0 hunks)token/TokenKeys.h(0 hunks)token/list/TokenList.c(1 hunks)token/token/token.c(1 hunks)util/CMakeLists.txt(0 hunks)
💤 Files with no reviewable changes (45)
- examples/typeinference/localvartypeinference.dg
- compiler/test/testcases.c
- tables/test/CMakeLists.txt
- stdlib/collections/stack.dg
- stdlib/collections/set.dg
- lexer/src/lexer_main.c
- lexer/test/test.h
- stdlib/collections/linkedlist.dg
- rat/CMakeLists.txt
- rat/test/CMakeLists.txt
- lexer/src/CMakeLists.txt
- stdlib/base/math.dg
- tac/CMakeLists.txt
- token/CMakeLists.txt
- compiler/main/typeinference/infer_in_context.c
- compiler/main/CMakeLists.txt
- ast/CMakeLists.txt
- compiler/main/gen_tac/gen_tac_forstmt.c
- parser/main/util/parser.c
- parser/test/CMakeLists.txt
- compiler/main/gen_tac/gen_tac_term.c
- ast/test/CMakeLists.txt
- util/CMakeLists.txt
- token/TokenKeys.h
- examples/tc_err.dg
- ibuffer/CMakeLists.txt
- tables/test/test.c
- tables/CMakeLists.txt
- parser/main/CMakeLists.txt
- stdlib/collections/array.dg
- compiler/main/x86_code_gen/compile_ir/compile_tac_unary_op.c
- tac/test/test_tac.c
- compiler/test/gen_tac/util/test_gen_tac_util.c
- parser/main/astnodes/types/Type.c
- compiler/main/gen_tac/gen_tac_assignstmt.c
- compiler/test/libvmcu_utils/libvmcu_utils.c
- lexer/test/CMakeLists.txt
- compiler/main/x86_code_gen/compile_ir/compile_tac_load_local_addr.c
- compiler/test/CMakeLists.txt
- compiler/main/avr_code_gen/compile_ir/compile_tac_binary_op.c
- lexer/test/testcases/tests_include_decl.c
- compiler/main/avr_code_gen/cg_avr_basic_block.c
- compiler/main/gen_tac/gen_tac_expr.c
- compiler/test/avr_code_gen/timer/test_avr_code_gen_timer.c
- compiler/main/x86_code_gen/compile_ir/compile_tac_binary_op.c
🧰 Additional context used
🧬 Code Graph Analysis (12)
tables/stst/stst_print.c (2)
tables/stst/stst.c (1)
stst_size(122-124)tables/stst/stst.h (1)
stst_size(39-39)
tables/sst/sst_print.c (2)
tables/sst/sst.c (1)
sst_size(180-182)tables/sst/sst.h (1)
sst_size(67-67)
ast/util/free_ast.c (1)
ast/util/free_ast.h (2)
free_enum_decl(32-32)free_method(22-22)
compiler/test/x86_code_gen/compile_ir/test_compile_tac_return.c (2)
compiler/test/x86_code_gen/test_x86_code_gen_util.c (1)
sd_uc_reg_read(536-538)compiler/test/x86_code_gen/test_x86_code_gen_util.h (1)
sd_uc_reg_read(35-35)
token/list/TokenList.c (1)
token/list/TokenList.h (2)
list_size(36-36)list_get(30-30)
compiler/main/cli/main.c (2)
rat/rat.c (1)
rat_ctor(21-43)rat/rat.h (1)
rat_ctor(18-18)
tac/tacbuffer.c (1)
tac/tacbuffer.h (2)
tacbuffer_set(27-27)tacbuffer_get(30-30)
rat/rat_x86.c (1)
rat/rat_x86.h (1)
rat_print_regname_x86(29-29)
compiler/main/x86_code_gen/allocate_registers_x86.c (3)
compiler/main/avr_code_gen/cg_avr_basic_block.c (1)
allocate_registers_single_tac(101-183)tac/tacbuffer.h (2)
tacbuffer_count(34-34)tacbuffer_get(30-30)tac/tacbuffer.c (2)
tacbuffer_count(122-124)tacbuffer_get(40-48)
parser/main/astnodes/Namespace.c (2)
token/list/TokenList.c (1)
list_copy(130-155)token/list/TokenList.h (1)
list_copy(24-24)
rat/rat.c (3)
rat/rat_x86.h (1)
rat_print_regname_x86(29-29)rat/rat_x86.c (1)
rat_print_regname_x86(203-208)rat/rat.h (2)
rat_capacity(73-73)rat_occupant(38-38)
lexer/src/lexer.c (2)
lexer/src/driver.h (1)
out_length(10-10)lexer/src/driver.c (1)
out_length(30-41)
🪛 CircleCI: build-job-ubuntu-2004
tables/stst/stst.c
[error] 158-158: Implicit declaration of function 'asprintf'; did you mean 'vsprintf'? (error due to -Werror=implicit-function-declaration)
tables/sst/sst.c
[error] 90-90: Implicit declaration of function 'asprintf'; did you mean 'vsprintf'? (error due to -Werror=implicit-function-declaration)
🪛 CircleCI: valgrind
tables/stst/stst.c
[error] 158-158: implicit declaration of function ‘asprintf’; did you mean ‘vsprintf’? (gcc -Werror=implicit-function-declaration)
tables/sst/sst.c
[error] 90-90: implicit declaration of function ‘asprintf’; did you mean ‘vsprintf’? (gcc -Werror=implicit-function-declaration)
🔇 Additional comments (82)
tac/tacbuffer_optimize.c (1)
11-11: Good addition to suppress compiler warnings.Adding
(void)debug;explicitly marks the unused parameter to prevent compiler warnings when using strict compilation flags like-Wall -Wextra -Werrormentioned in the PR title.parser/main/astnodes/var/SimpleVar.c (1)
72-72: LGTM: Clean whitespace removal.This change simply removes an unnecessary blank line, improving code readability.
compiler/main/avr_code_gen/compile_ir/compile_tac_const_value.c (1)
19-19: Good buffer size increase to prevent potential overflows.Increasing the character buffer size from 30 to 60 bytes helps prevent buffer overflow when formatting debug strings with longer constant values.
lexer/test/testcases/tests_other.c (1)
228-228: Good type consistency improvement.Changing the loop index from
inttosize_taligns with the type ofexpect_count, improving type safety by preventing potential signed/unsigned comparison issues.tables/symtable/symtable.c (1)
50-50: Good type consistency fix.Changing the loop index from
inttosize_tproperly aligns with the type ofst->inferred_types_count, preventing potential issues with signed/unsigned comparisons during iteration.compiler/test/avr_code_gen/compile_ir/test_compile_tac_store.c (1)
49-49: Good addition - missing assertion was added.This assertion ensures the test verifies that the stored value matches the expected value, which all other similar tests already had. This improves test coverage and consistency.
examples/stdlib/syscalls/write_file/write_file.dg (1)
5-5: Improved to standard octal notation for file permissions.The change from hexadecimal to octal notation (0666) for file permissions follows Unix/Linux convention and improves readability. Both 0x1B6 and 0666 represent the same permissions (rw-rw-rw-), but octal is the standard format for representing file modes.
compiler/test/x86_code_gen/fake_lvst.c (1)
55-55: Good type consistency improvement.Changed loop counters from
inttosize_tto match the type ofnlocalsandstackframe_nargs. This prevents potential signed/unsigned comparison warnings and follows best practices for iteration variables.Also applies to: 58-58
CMakeLists.txt (1)
11-11: Simplified compiler flags application.Replaced variable-based flags with direct application using
add_compile_options. The flags enable:
- Debug symbols (
-g)- CPU architecture optimization (
-march=native)- Comprehensive warnings (
-Wall -Wextra)- Treating warnings as errors (
-Werror)This enforces strict code quality standards across the project.
examples/octal/octal_notation.exitcode (1)
1-2: Exit code file looks goodThis file correctly specifies the expected exit code (54) for the octal notation example program, which aligns with the PR objective of adding octal notation support.
parser/main/astnodes/expr/Term.c (1)
96-100: Control flow simplified by removing unused labelThe removal of the
exit_error:label simplifies the control flow while preserving the same error handling logic. This is a good cleanup that aligns with the overall code simplification goals in the PR.tables/stst/stst_print.c (1)
18-18: Improved type safety for loop indexChanging the loop index from
inttosize_timproves type safety by using the appropriate unsigned type for array indices. This aligns with the return type ofstst_size()(which isuint32_t) and prevents potential signed/unsigned comparison issues.compiler/main/x86_code_gen/compile_ir/compile_tac_icall.c (1)
17-18: Good practice: Explicit unused parameter markingAdding the
(void)casts to explicitly mark unused parameters is a good practice when working with strict compiler flags like-Wall -Wextra -Werror. This suppresses unused parameter warnings without changing the function behavior.compiler/main/compiler.c (3)
63-63: Loop index type aligned with size_t count variableChanging the loop index from
inttosize_tproperly aligns the types and prevents potential issues with signed/unsigned comparisons, sincecount_filenamesis declared assize_t.
93-93: Consistent size_t loop index for array iterationGood consistency in changing this loop index to
size_tto match the type of the count variable and the previous loop.
168-168: Completed size_t conversion for all loopsThe conversion of all loop indices in this file to
size_tensures consistent type usage throughout the token list processing loops.tables/sst/sst_print.c (1)
20-20: Improved type safety with size_t loop indexChanging the loop index from
inttosize_teliminates potential signed/unsigned comparison issues sincesst_size()returns auint32_tvalue.tables/sst/sst_fill.c (1)
11-11: Loop index type matches Namespace count_methods typeChanging the loop index from
inttosize_tcorrectly aligns with the updated type ofns->count_methodswhich was changed fromuint16_ttosize_tin the Namespace struct.compiler/main/avr_code_gen/compile_ir/compile_tac_label.c (1)
27-27: Type safety improvement for better comparisonThe cast of
tac_dest(tac)tosize_tensures consistent unsigned comparison withsst_size(), which likely returns an unsigned size type. This eliminates potential issues from comparing signed and unsigned values.compiler/main/x86_code_gen/compile_ir/compile_tac_label.c (1)
27-27: Type safety improvement in comparison logicThe cast of
sst_size()toint32_tensures a consistent signed comparison withtac_dest(tac). This approach differs from the AVR implementation (which casts to unsigned) but both achieve the same goal of type-consistent comparisons.compiler/main/analyzer/dead/dead_analyzer.c (1)
60-60: Improved type safety for loop counterChanging the loop variable type from
inttosize_tis appropriate when iterating over collection sizes. This matches the likely type ofns->count_methodsand prevents potential issues with signed/unsigned comparisons.compiler/main/x86_code_gen/compile_ir/compile_tac_call.c (1)
21-22: Suppressing unused parameter warningExplicitly marking the unused parameter with
(void)current_function_name;is a good practice to silence compiler warnings while maintaining the function signature for consistency with other implementations.token/token/token.c (1)
75-75: Use of size_t improves type safety for string iterationChanging the loop index from
inttosize_tis a good improvement as it properly matches the type oflen(which is already defined assize_ton line 73). This change avoids potential signed/unsigned comparison issues when iterating over string data.ast/util/free_ast.c (1)
61-62: Type consistency improvement for loop indicesChanging the loop counters from
inttosize_tis a positive improvement that makes the code more type-safe when working with the count fields in the Namespace struct. This change helps prevent potential signed/unsigned comparison issues and aligns with modern C programming practices.Also applies to: 65-66, 69-70
compiler/test/x86_code_gen/compile_ir/test_compile_tac_return.c (2)
43-43: Added critical error checking for emulator initializationAdding this assertion is a good defensive programming practice. It ensures that the Unicorn emulator successfully starts before continuing with the test, making the test more robust by failing early if there's an initialization problem.
48-48: Added critical error checking for register read operationThis assertion properly validates that the register read operation succeeded before checking the register value. This makes the test more robust by ensuring that failures in the emulator API are caught explicitly rather than potentially leading to misleading test results.
rat/rat_x86.h (1)
29-29: Function signature simplified by removing unused parameterThe removal of the
struct RAT* ratparameter simplifies the function interface. This is a good refactoring since it makes the function more focused on its actual responsibility - printing an x86 register name. The removed parameter was likely unnecessary as the function can determine the register name solely from the register index.examples/octal/octal_notation.dg (1)
1-11: LGTM! Good example of octal notationThis is a clean, concise example demonstrating the use of octal notation in the language. The function correctly initializes a uint64 with octal literal
0666(438 decimal), subtracts0600(384 decimal), and returns the result066(54 decimal).token/list/TokenList.c (2)
265-265: Improved token list dumping by removing the 10 token limitThe function now iterates through all tokens in the list instead of just the first 10, which aligns with the removal of special token handling across the codebase.
271-271: Better token value formatting with space prefixAdding a space before printing the token's value improves readability in the output by ensuring proper separation between token kind and value.
compiler/test/gen_tac/test_gen_tac_simplevar.c (2)
85-85: Increased buffer size for safer string operationsIncreasing the snippet buffer from 200 to 400 bytes helps prevent potential buffer overflows when dealing with larger code fragments.
87-87: Increased buffer size for safer string operationsIncreasing the snippet2 buffer from 200 to 300 bytes helps prevent potential buffer overflows when dealing with larger code fragments.
tables/stst/stst.c (3)
52-52: Improved type safety with size_t for array indexingChanging the loop index from
inttosize_tensures type consistency with the unsignedcount_structsvariable used for array indexing, preventing potential signed/unsigned comparison issues.
64-64: Improved type safety with size_t for array indexingChanging the loop index from
inttosize_tensures type consistency with the unsignedcountvariable used for array indexing, preventing potential signed/unsigned comparison issues.
134-134: Improved type safety with size_t for array indexingChanging the loop index from
inttosize_tensures type consistency with the unsignedcountvariable used for array indexing, preventing potential signed/unsigned comparison issues.compiler/main/cli/main.c (1)
22-22: Architecture selection improvementThe code now passes the dynamically selected architecture to
rat_ctorinstead of hardcodingRAT_ARCH_X86. This aligns with the earlier architecture determination in the function wherearchis conditionally set based on flags.ast/visitor/visitor.c (3)
86-86: Type safety improvement for loop countersChanging from
inttosize_tfor the loop counter aligns with the type ofn->count_enumsand follows best practices for array indexing, preventing potential issues with large arrays and negative indices.
90-90: Type safety improvement for loop countersChanging from
inttosize_tfor the loop counter aligns with the type ofn->count_structsand follows best practices for array indexing.
94-94: Type safety improvement for loop countersChanging from
inttosize_tfor the loop counter aligns with the type ofn->count_methodsand follows best practices for array indexing.compiler/main/gen_tac/gen_tac_call.c (1)
30-30: Type safety improvement for loop countersChanged from
inttosize_tfor the loop counter to match thecountparameter type, avoiding potential signed/unsigned comparison issues and maintaining consistency with other similar changes throughout the codebase.ast/util/str_ast.c (2)
306-306: Increased buffer size for type parametersIncreased malloc size from 4 to 6 bytes, likely to accommodate larger formatted output in the
sprintfcall at line 311. This prevents potential buffer overflow issues when formatting larger type parameter indices.
639-639: Added support for enum value termsAdded missing case to handle
TERM_KIND_ENUM_VALUEby returning a duplicate of the enum value string. This properly handles enum values in the AST string representation, completing the switch statement implementation.compiler/test/x86_code_gen/compile_ir/test_compile_tac_unary_op.c (1)
37-37: Good type safety fix with explicit cast.Adding the explicit cast to
intensures proper signed comparison with the negatedstartvalue. This prevents potential assertion failures when comparing unsigneduint64_twith signed values.compiler/test/x86_code_gen/compile_ir/test_compile_tac_if_cmp_goto.c (1)
19-20: Improved type consistency.Changing these constants from
int8_ttouint64_tproperly aligns them with the 64-bit register and memory operations used in the tests. This prevents potential issues with type promotion during comparisons on lines 70-75.compiler/test/x86_code_gen/compile_ir/test_compile_tac_if_goto.c (2)
19-19: Type consistency improvement.Changing from
int8_ttouint64_taligns with the overall pattern of using consistent types for register values and ensures proper comparison with thereadvariable on line 50.
89-89: Explicit casts prevent signed/unsigned comparison issues.Adding explicit casts to
intensures the comparison is done between values of the same signedness, preventing unexpected behavior when comparing signed and unsigned values.ast/ast.h (2)
42-42: Appropriate type changes for count fields.Changing from
uint16_ttosize_tfor count variables is a good practice.size_tis the standard type for sizes and counts in C, and this change aligns with the use ofsize_tfor the corresponding capacity fields.This also prepares the code for potentially larger collections and ensures consistency with loop counters elsewhere in the codebase.
Also applies to: 47-48, 51-52
31-53: Removed include-related fields from Namespace struct.The removal of
count_includesandincludesfields from the Namespace struct is consistent with the PR's goal of removing support for handling#includedirectives. This simplifies the AST structure.compiler/test/x86_code_gen/compile_ir/test_compile_tac_binary_op.c (4)
42-42: Type cast ensures consistent comparison.The addition of the
(int)cast properly ensures that the 64-bit register value is compared correctly with the 8-bit expected value. This fix prevents potential sign extension issues when comparing different integer sizes.
74-74: Type cast ensures consistent comparison.Similarly, this cast properly handles the comparison between the 64-bit register value and the 8-bit expected value, ensuring sign is correctly interpreted.
105-105: Type cast ensures consistent comparison.This cast addresses the same issue - ensuring proper comparison between the 64-bit
raxregister value and the 8-bit expected value in a bitwise AND operation.
136-136: Type cast ensures consistent comparison.This type cast provides consistency with the other binary operations, ensuring proper comparison between different integer sizes in the OR operation test.
rat/rat_x86.c (1)
203-203: Function signature simplified by removing unused parameter.The
struct RAT* ratparameter has been removed from the function signature, which is a good simplification since the parameter wasn't being used. This matches the declaration inrat/rat_x86.h.tables/sst/sst.c (6)
54-54: Loop index type changed to match container size type.Changing the loop index from
inttouint32_tto match the type ofsst->count(which isuint32_t) prevents potential signed/unsigned comparison warnings.
69-69: Loop index type changed to match container size type.Similarly, this change aligns the loop counter type with
sst->countfor type consistency.
85-86: Explicitly cast unused parameter to void.This change properly suppresses compiler warnings about the unused
isLibCparameter.
169-169: Loop index type changed to match container size type.This change to
uint32_tmaintains consistency with the type ofsst->count.
195-195: Loop index type changed to match container size type.Using
size_tinstead ofintfor the loop counter is appropriate when iterating through array indices.
226-226: Loop index type changed to match container size type.Using
size_tfor index variables is a good practice when iterating through arrays.tac/tacbuffer.c (3)
29-29: Parameter type changed from int to size_t.Changing the
indexparameter type frominttosize_taligns with the function declaration in the header file and is more appropriate for array indices which should always be non-negative.
40-40: Parameter type changed from int to size_t.Similarly, this type change ensures consistency between the function implementation and its declaration in the header.
43-43: Format specifier updated to match size_t.The format specifier has been correctly updated from
%dto%ldto match thesize_ttype of the index parameter.compiler/main/basic_block/basicblock.c (2)
81-81: Good explicit type cast for signedness compatibility.Adding this explicit cast to
ssize_tensures proper comparison between the signed return value fromtacbuffer_indexofand the unsigned result oftacbuffer_count(buffer) - 1, preventing potential issues with signed/unsigned comparisons.
100-100: Appropriate cast for comparing different integer types.This explicit cast from
uint32_ttoint32_tensures type correctness when comparing with the result oftac_label_index(). This prevents potential compiler warnings and subtle bugs from signed/unsigned comparisons.compiler/test/x86_code_gen/test_x86_code_gen_util.c (6)
57-57: Good explicit void cast for unused parameter.Marking the unused parameter with
(void)suppresses compiler warnings while making the intention clear that this parameter is deliberately unused in the function.
72-73: Good explicit void casts for unused parameters.Properly marking both unused parameters with
(void)casts suppresses compiler warnings. This is especially important when compiling with-Wall -Wextraflags.
191-191: Improved loop index type from int to size_t.Using
size_tfor array iteration is more appropriate since you're comparing against the result of asizeofexpression, which returnssize_t. This eliminates potential signed/unsigned comparison warnings.
350-350: Appropriate use of size_t for loop counter.Changed from
inttosize_tto match the type ofnargs, which is consistent with best practices for iterating over collections with non-negative indices.
372-372: Proper index type for array iteration.Using
size_tinstead ofintfor the loop counter matches the type ofnargsand follows best practices for array iteration with non-negative indices.
565-565: Consistent use of size_t for array iteration.Using
size_tfor the loop index is appropriate when calculating array length withsizeof. This provides type consistency and prevents signed/unsigned comparison warnings.tac/tacbuffer.h (2)
27-27: Improved parameter type for array indexing.Changed
indexparameter frominttosize_t, which is the appropriate type for array indices. This provides better type safety and consistency with the standard library.
30-30: Consistent use of size_t for index parameter.The parameter type change from
inttosize_tfor the index aligns with best practices for array indexing and maintains consistency with the tacbuffer_set function's signature change.compiler/main/x86_code_gen/allocate_registers_x86.c (4)
16-16: Simplified function signature by removing unused parameter.Removing the unused
struct ST* stparameter from the function declaration reduces the function's complexity and dependency on the symbol table structure.
32-33: Properly marked unused parameter with void cast.Adding
(void)st;explicitly acknowledges that the parameter is deliberately unused, which suppresses compiler warnings when building with-Wall -Wextra -Werrorflags.
39-39: Updated function call to match new signature.The function call has been updated to remove the
stparameter, maintaining consistency with the new function signature defined on line 16.
88-88: Updated function implementation to match new declaration.The function implementation has been correctly updated to match the new signature without the
struct ST* stparameter, consistent with its declaration on line 16.rat/rat.c (6)
80-80: Function call updated to match new signatureThe call to
rat_print_regname_x86has been updated to remove theratparameter, matching the new function signature inrat/rat_x86.c(line 203). This simplifies the interface by removing an unnecessary parameter that wasn't being used in the implementation.
268-278: Improved boundary checking and logic inrat_is_wideThe rewritten function now has better boundary and occupancy checks with a more logical flow:
- Check if the next register is within bounds
- Verify that the next register is occupied
- Confirm that the occupant is the same temporary variable
This is clearer than the previous implementation and correctly identifies when a temporary uses a register pair.
350-351: Added explicit default return valueAdding an explicit default return value for
rat_scratch_regensures the function always returns a value even if the architecture is not handled in the switch statement. This is good defensive programming that prevents undefined behavior.
362-363: Added explicit default return valueSimilar to the previous change, adding a default return for
rat_return_regensures proper handling of unexpected architecture values.
386-387: Added explicit default return valueAdding an explicit default return value for
rat_stack_ptris consistent with the other changes and prevents undefined behavior.
392-398: Architecture-specific register capacity handlingThe
rat_capacityfunction now returns architecture-specific register capacity constants (SD_REG_END_X86for x86 andSD_REGISTER_ENDfor AVR), which correctly reflects the actual register constraints for each architecture.This is a better approach than using a hardcoded value, as it makes the code more maintainable and adaptable to changes in the register definitions for each architecture.
pointbazaar
force-pushed
the
octal-notation
branch
11 times, most recently
from
8e6d4fa to
5dc935e
Compare
pointbazaar
force-pushed
the
octal-notation
branch
from
5dc935e to
bab4db9
Compare
Fixes for issues found in code review
1 participant
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
#includedirectives.Documentation
Tests
#includedirective handling.Chores