feat: add StructDefStmt LLVM IR codegen #150
feat: add StructDefStmt LLVM IR codegen #150omsherikar wants to merge 2 commits intoarxlang:mainfrom
Conversation
OSL ChatGPT ReviewerNOTE: This is generated by an AI program, so some comments may not make sense. src/irx/builders/llvmliteir.py
|
There was a problem hiding this comment.
Pull request overview
This PR implements LLVM IR code generation for astx.StructDefStmt to enable struct type definitions in the IRX compiler. The changes add infrastructure to track struct types and their definitions for future use in struct instantiation and field access operations.
Key Changes:
- Added struct tracking dictionaries (
struct_typesandstruct_defs) to theLLVMLiteIRVisitorclass - Implemented
visit(StructDefStmt)method to generate LLVMIdentifiedStructTypedefinitions - Converts ASTx struct attributes to LLVM field types using existing type mapping infrastructure
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| field_types = [] | ||
| for attr in node.attributes: | ||
| type_str = attr.type_.__class__.__name__.lower() | ||
| field_type = self._llvm.get_data_type(type_str) |
There was a problem hiding this comment.
The type conversion logic using attr.type_.class.name.lower() may fail if get_data_type doesn't recognize the type name. When get_data_type encounters an unknown type at line 145, it raises an exception with "[EE]: Type name {type_name} not valid." However, this could be misleading for struct field types. Consider adding a try-except block or pre-validating the type to provide a more specific error message indicating which field in which struct has an unsupported type.
| field_type = self._llvm.get_data_type(type_str) | |
| try: | |
| field_type = self._llvm.get_data_type(type_str) | |
| except Exception as e: | |
| # Provide more context for unsupported struct field types. | |
| field_name = getattr(attr, "name", "<unknown>") | |
| raise Exception( | |
| f"Unsupported type '{type_str}' for field '{field_name}' " | |
| f"in struct '{node.name}'." | |
| ) from e |
| field_type = self._llvm.get_data_type(type_str) | ||
| field_types.append(field_type) | ||
|
|
||
| struct_type.set_body(*field_types) |
There was a problem hiding this comment.
The implementation doesn't handle the case where node.attributes is empty. Creating a struct with no fields will result in calling struct_type.set_body() with no arguments, which may not be the intended behavior for LLVM IR. Consider adding a check to handle empty structs or document this behavior if it's intentional.
| struct_type.set_body(*field_types) | |
| if field_types: | |
| # Struct with one or more fields. | |
| struct_type.set_body(*field_types) | |
| else: | |
| # Intentionally define an empty struct when there are no attributes. | |
| struct_type.set_body() |
| field_type = self._llvm.get_data_type(type_str) | ||
| field_types.append(field_type) | ||
|
|
||
| struct_type.set_body(*field_types) |
There was a problem hiding this comment.
The visit method for StructDefStmt does not append to result_stack. Other similar visitor methods (like visit for FunctionDef at line 1919 and FunctionPrototype at line 1958) append their results to result_stack. This could cause issues if code expects to retrieve the struct type from the result stack. Consider appending struct_type to result_stack for consistency.
| struct_type.set_body(*field_types) | |
| struct_type.set_body(*field_types) | |
| self.result_stack.append(struct_type) |
| def visit(self, node: astx.StructDefStmt) -> None: | ||
| """Translate ASTx StructDefStmt to LLVM-IR.""" | ||
| if node.name in self.struct_types: | ||
| raise Exception(f"Struct '{node.name}' already defined.") |
There was a problem hiding this comment.
The error message format is inconsistent with other error messages in the codebase. Most error messages use a prefix like "[EE]:" (see line 145) or "codegen:" (see lines 500, 507, 512, etc.). Consider using a consistent prefix format such as "codegen: Struct 'name' already defined." for better error tracking and consistency.
| raise Exception(f"Struct '{node.name}' already defined.") | |
| raise Exception(f"codegen: Struct '{node.name}' already defined.") |
|
@xmnlab @yuvimittal please have a look |
|
@xmnlab @yuvimittal can you please look into it? |
OSL ChatGPT ReviewerNOTE: This is generated by an AI program, so some comments may not make sense. src/irx/builders/llvmliteir.pyChatGPT was not able to review the file. Error: Error code: 429 - {'error': {'message': 'You exceeded your current quota, please check your plan and billing details. For more information on this error, read the docs: https://platform.openai.com/docs/guides/error-codes/api-errors.', 'type': 'insufficient_quota', 'param': None, 'code': 'insufficient_quota'}} |
Pull Request description
Implements LLVM IR codegen for
astx.StructDefStmtto generateIdentifiedStructTypedefinitions. Adds struct tracking dictionaries (struct_typesandstruct_defs) to track struct definitions for future use in struct instantiation and field access.Fixes #143
How to test these changes
StructDefStmtwith attributes (e.g., Point with x, y as Int32)LLVMLiteIR().translator.translate(module)%"Point" = type {i32, i32}Pull Request checklists
This PR is a:
About this PR:
Author's checklist:
complexity.
Additional information
Example LLVM IR output:vm
%"Point" = type {i32, i32}## Reviewer's checklist
Copy and paste this template for your review's note: