Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

migrate to infix bitwise operators and string builder #65

Merged
merged 4 commits into from
Oct 18, 2024

Conversation

Yoorkin
Copy link
Collaborator

@Yoorkin Yoorkin commented Oct 18, 2024

No description provided.

Copy link

peter-jerry-ye-code-review bot commented Oct 18, 2024

From the provided git diff output, here are three potential issues or areas of concern across the modifications:

  1. Bitwise Operation Syntax:

    • The changes in multiple files (e.g., crypto/chacha.mbt, crypto/md5.mbt, crypto/sha1.mbt, etc.) replace .lsl() and .lsr() with << and >> respectively. While both forms achieve the same bitwise shift operations, the use of << and >> is more conventional in many programming languages. However, it's important to ensure that the MoonBit language specification and compiler correctly support these operators for the intended types (e.g., UInt, Int, etc.).

    Suggestion: Verify that the MoonBit compiler and language specification support the << and >> operators correctly for the intended data types to avoid any unexpected behavior or bugs.

  2. Type Conversions and Bit Manipulations:

    • In several files (e.g., crypto/md5.mbt, crypto/sha1.mbt, crypto/sha256.mbt), there are changes involving bitwise operations and type conversions (e.g., .reinterpret_as_int(), .to_int64(), .to_byte(), etc.). These changes could potentially introduce subtle bugs if the reinterpretation or conversion does not handle the underlying bit representation correctly.

    Suggestion: Carefully review the affected sections to ensure that the type conversions and bit manipulations are correctly implemented and do not alter the intended logic or data representation.

  3. Buffer and StringBuilder Usage:

    • Changes in multiple files (e.g., fs/internal/ffi/string_wasm.mbt, time/duration.mbt, time/plain_date.mbt, etc.) replace Buffer::new() with StringBuilder::new(). This change suggests a switch from using a generic buffer to a string builder, which is typically optimized for string concatenation operations.

    Suggestion: Ensure that the StringBuilder class behaves as expected in terms of performance and functionality, especially if the Buffer class previously provided specific behaviors or optimizations that are now expected from StringBuilder. Thoroughly test these changes to confirm that they do not introduce regressions in the expected output or performance characteristics.

In summary, while the changes in the git diff output might seem straightforward, they touch upon fundamental aspects of the codebase, including bitwise operations, type conversions, and data structure usage. It's crucial to ensure that these changes align with the MoonBit language specification and do not introduce new bugs or inconsistencies.

@Yoorkin Yoorkin merged commit 59b2920 into main Oct 18, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant