runtime: math/big use math_big_pure_go

[visualfc]

use math_big_pure_go for full support math/big

[niupilot]

PR Review: runtime: math/big use math_big_pure_go

Todo List:

• Examine the changed files to understand current state
• Analyze git diff to see what was removed/changed
• Review the architectural decision and its implications
• Provide comprehensive code review feedback

Summary

This PR makes a significant architectural change by removing the custom OpenSSL-based math/big implementation and switching to Go's pure Go implementation using the math_big_pure_go build tag.

Changes Analysis

1. Build Configuration (internal/build/build.go:219)

• Change: Added math_big_pure_go to the default build tags
• Impact: Forces use of Go's pure Go math/big implementation instead of CGO/OpenSSL version

2. Runtime Configuration (runtime/build.go:43)

• Change: Removed "math/big" from hasAltPkg map
• Impact: No longer treats math/big as a package requiring alternative implementation

3. Removed Custom Implementation

Deleted 554 lines of custom math/big implementation across:

• runtime/internal/lib/math/big/int.go (458 lines) - Core integer operations using OpenSSL BIGNUM
• runtime/internal/lib/math/big/intconv.go (70 lines) - String conversion operations
• runtime/internal/lib/math/big/prime.go (26 lines) - Primality testing

Review Findings

❌ Critical Issues

1. Incomplete Implementation Removal (runtime/internal/lib/math/big/int.go)

   • The removed custom implementation had many unimplemented functions marked with panic("todo ...")
   • Functions like Mul, Quo, Div, Mod, SetString, Bytes, BitLen, etc. were incomplete
   • Risk: This suggests the custom implementation was not production-ready

2. Missing Documentation/Justification

   • No explanation for why this architectural change was made
   • No performance comparison between OpenSSL-based and pure Go implementations
   • No discussion of trade-offs (performance vs dependencies)

⚠️ Design Concerns

3. Build Tag Strategy (internal/build/build.go:219)

   • Adding math_big_pure_go as a default build tag is a global decision
   • This affects all builds, not just specific scenarios where pure Go is preferred
   • Suggestion: Consider making this configurable rather than always-on

4. Dependency Reduction vs Performance

   • Pro: Eliminates OpenSSL dependency, improving build simplicity
   • Con: Pure Go math/big is generally slower than optimized C implementations
   • Missing: No performance benchmarks or analysis provided

✅ Positive Aspects

5. Code Cleanup

   • Removes substantial amount of incomplete/placeholder code
   • Simplifies the build system by removing custom math/big handling
   • Eliminates CGO dependency for math/big operations

6. Implementation Consistency

   • Using Go's standard library ensures compatibility and correctness
   • Leverages well-tested, mature implementation
   • Reduces maintenance burden

Recommendations

1. Add Documentation: Include rationale for this change in commit message or PR description
2. Performance Analysis: Provide benchmarks comparing OpenSSL vs pure Go performance
3. Consider Configurability: Make this choice configurable rather than hardcoded
4. Integration Testing: Ensure all math/big usage in the codebase works correctly with pure Go implementation

Verdict

This change simplifies the architecture and removes incomplete code, but lacks proper justification and performance analysis. The direction is sound, but the implementation needs better documentation and consideration of configurability.

---

👉 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 streamlines the math/big package by adopting a pure Go implementation. This change eliminates the reliance on external C libraries, such as OpenSSL, for arbitrary-precision arithmetic, which simplifies the build process, reduces external dependencies, and enhances portability across different environments.

Highlights

• Pure Go math/big Implementation: The pull request transitions the math/big package to utilize a pure Go implementation, moving away from external C library dependencies.
• Removal of OpenSSL-dependent Files: Several files (int.go, intconv.go, prime.go) that provided an OpenSSL-backed implementation for math/big functionality have been removed.
• Build Tag Integration: A new build tag, math_big_pure_go, has been added to the build configuration, enabling the selection of the pure Go version of the math/big package.
• Alternative Package Listing Update: The math/big package has been removed from the list of packages with alternative implementations, indicating a consolidated approach to its runtime.

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 replaces the custom, OpenSSL-based implementation of math/big with the standard library's pure Go implementation. This is achieved by removing the alternative implementation files and adding the math_big_pure_go build tag. This is an excellent change that improves correctness and maintainability by removing an incomplete, C-dependent implementation in favor of a complete and portable one from the standard library. The changes are consistent and well-executed.

[niupilot]

This PR removes the incomplete OpenSSL-based math/big implementation in favor of Go's pure Go implementation. While this simplifies the codebase and eliminates CGO dependencies, there are some considerations for improvement.

Key Issues:
• Missing documentation explaining the rationale for this architectural change
• No performance impact analysis between OpenSSL and pure Go implementations
• Build tag is hardcoded rather than configurable

Positive Changes:
• Removes 554 lines of incomplete/placeholder code with many panic("todo") functions
• Eliminates OpenSSL dependency and CGO complexity
• Uses well-tested standard library implementation

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.14%. Comparing base (8959c83) to head (62d0ee3).
⚠️ Report is 9 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1323      +/-   ##
==========================================
+ Coverage   90.10%   90.14%   +0.03%     
==========================================
  Files          43       43              
  Lines       12576    12576              
==========================================
+ Hits        11332    11337       +5     
+ Misses       1088     1083       -5     
  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.

[xushiwei]

don't need to define math_big_pure_go tag