Feature: Type deduction struct field ptr inst - #823
LindonAliu wants to merge 2 commits into
Conversation
7a8c166 to
eada721
Compare
There was a problem hiding this comment.
🟡 Not ready to approve
There’s a mismatch between the stated #435 goal (target-aware pointer size/alignment deduction) and what the current changes demonstrably implement, plus a small but actionable assert-message improvement is needed.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
This PR aims to improve/lock in correct type deduction for StructFieldPtrInst results (referenced as fixing #435) by ensuring the instruction’s result type is a pointer to the accessed field type and adding a regression test around that behavior.
Changes:
- Added a GILGen unit test asserting
StructFieldPtrInstresult types point to the member field type. - Refactored
buildStructFieldPtrto usegetASTContext()when allocating thePointerTy. - Added an invariant check in
StructFieldPtrInstverifying the provided pointer type’s pointee matches the member type.
File summaries
| File | Description |
|---|---|
| test/GILGen/GILGenStmt.cpp | Adds a regression test that validates StructFieldPtrInst result type pointee matches the field type. |
| include/GILGen/Context.hpp | Refactors pointer type allocation to go through getASTContext() when building StructFieldPtrInst. |
| include/GIL/Instructions/Aggregates/StructFieldPtrInst.hpp | Adds a pointee-type invariant assert and required include for PointerTy. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 2
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
| auto *ptrType = llvm::cast<types::PointerTy>(pointerType); | ||
| assert(ptrType->getPointee() == member.getType()); |
| auto *fieldPtrType = getASTContext() | ||
| ->getTypesMemoryArena() | ||
| .create<glu::types::PointerTy>(member.getType() | ||
| .create<glu::types::PointerTy>( | ||
| member.getType() | ||
| ); |
fix #435