ssa: fix abiTupleOf

[visualfc]

fix interface abi load order #1324

[niupilot]

PR Review: ssa: fix abiTupleOf

Todo List:

• Read and analyze the core changes in ssa/abitype.go
• Understand what issue segmentation fault for go/build #1324 was about and how this fixes it
• Review code quality and identify potential issues
• Provide comprehensive review feedback

---

Summary

This PR fixes a critical bug in the abiTupleOf function that was causing segmentation faults when importing certain packages like go/build in different orders. The issue was that the function was storing Expr values directly instead of storing closures that generate Expr values when needed.

Code Analysis

The fix in ssa/abitype.go:87-102 addresses a fundamental problem with lazy evaluation:

Before (problematic):

tuple := make([]Expr, n)
for i := 0; i < n; i++ {
    tuple[i] = b.abiType(t.At(i).Type())  // Immediate evaluation
}
return func() Expr {
    // ...
    return b.SliceLit(tSlice, tuple...)  // Uses pre-evaluated expressions
}

After (fixed):

tuple := make([]func() Expr, n)
for i := 0; i < n; i++ {
    tuple[i] = b.abiTypeOf(t.At(i).Type())  // Stores closure for lazy evaluation
}
return func() Expr {
    // ...
    elts := make([]Expr, n)
    for i := 0; i < n; i++ {
        elts[i] = tuple[i]()  // Evaluates closures when needed
    }
    return b.SliceLit(tSlice, elts...)
}

Issues Identified

🚨 High Confidence Issues

1. Inconsistent API Usage: The original code mixed b.abiType() (immediate) with b.abiTypeOf() (lazy) patterns, breaking the lazy evaluation design that prevents circular dependencies in ABI type construction.

2. Import Order Dependency: The premature evaluation caused segfaults when complex types with circular references were processed in certain import orders, as seen in issue segmentation fault for go/build #1324.

3. Memory Safety: The segmentation faults indicate potential stack overflow or null pointer dereference from circular evaluation chains.

Verification

The fix is validated by:

• 13 test files updated: All .ll output files show consistent changes across test cases
• Pattern consistency: The fix aligns with similar lazy evaluation patterns used elsewhere in the codebase (e.g., abiPointerOf, abiSliceOf)
• API coherence: Now consistently uses abiTypeOf() for lazy evaluation like other composite type handlers

Impact Assessment

✅ Positive Impact:

• Resolves segmentation faults in complex import scenarios
• Maintains consistent lazy evaluation pattern throughout the ABI system
• Fixes import order dependencies that could cause unpredictable runtime behavior

⚠️ Potential Concerns:

• Large diff in test files (+2850/-2750) suggests significant changes to generated output, though this appears to be expected from the ABI fix

Conclusion

This is a well-targeted fix that addresses the root cause of the segmentation fault issue. The change maintains the existing API contract while fixing the premature evaluation bug. The solution follows established patterns in the codebase and should resolve the import order dependency issues reported in #1324.

Recommendation: ✅ APPROVE - This fix correctly addresses a critical bug with a clean, pattern-consistent solution.

---

👉 Want me to re-run this review? Just type /review in the comments! For more usage examples, visit CodeAgent GitHub Repository.

[gemini-code-assist]

Summary of Changes

Hello @visualfc, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses a critical bug in the SSA phase of the compiler related to the abiTupleOf function. The core issue was an incorrect handling of type expression evaluation for tuples, which could lead to malformed LLVM IR. The changes introduce a deferred evaluation mechanism for type expressions, ensuring that type information is correctly assembled when generating the final LLVM output. This resolves a reported issue and improves the robustness of the type system's representation in the generated code.

Highlights

• ssa: abiTupleOf Function Fix: The abiTupleOf function in ssa/abitype.go has been refactored to correctly handle the generation of ABI type representations for tuples. Previously, it might have prematurely evaluated type expressions, leading to incorrect LLVM IR.
• Deferred Type Expression Evaluation: The fix involves changing the tuple slice to store functions that return Expr (type expressions) instead of Expr directly. This defers the evaluation of these expressions until they are needed, ensuring proper context and resolution during LLVM IR generation.
• LLVM IR Output Adjustments: Several test output files (.ll files) have been updated to reflect the changes in how type information is processed. These modifications include reordering of load instructions and the introduction of runtime.Basic or runtime.NewNamedInterface calls, indicating a more precise construction of type metadata.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request addresses a bug in ssa.Builder.abiTupleOf by switching from eager to lazy evaluation for ABI type generation of tuple elements. The change from b.abiType to b.abiTypeOf correctly defers expression generation by using thunks (func() Expr), which are evaluated later within the returned closure. This ensures that code is generated in the correct context. The fix is well-implemented and improves the correctness of the SSA package. The updates to the LLVM IR test files are consistent with this change.

[niupilot]

Code Review Summary

This PR correctly fixes a critical bug in abiTupleOf that was causing segmentation faults due to premature evaluation of ABI types. The fix implements proper lazy evaluation using closures, which aligns with the established patterns in the codebase.

Key Improvements:

• ✅ Fixes import order dependency segfaults reported in issue #1324
• ✅ Maintains consistent lazy evaluation pattern throughout ABI system
• ✅ Follows established patterns used in other composite type handlers
• ✅ Preserves existing API contract while fixing the underlying bug

The large test file changes (+2850/-2750) appear to be the expected result of fixing the ABI generation, indicating comprehensive test coverage validation.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.11%. Comparing base (8959c83) to head (5592a8f).
⚠️ Report is 13 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1327   +/-   ##
=======================================
  Coverage   90.10%   90.11%           
=======================================
  Files          43       43           
  Lines       12576    12579    +3     
=======================================
+ Hits        11332    11335    +3     
  Misses       1088     1088           
  Partials      156      156           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.