Skip to content

Conversation

@Karesis
Copy link
Contributor

@Karesis Karesis commented Dec 4, 2025

Description

This PR implements the get_alignment method in the BasicType trait by utilizing the underlying Type::get_alignment implementation.

Previously, get_alignment was only available on specific types (like IntType, StructType) but not accessible via the BasicType trait or BasicTypeEnum. This change aligns the behavior with size_of, allowing generic usage of alignment retrieval.

Related Issue

N/A (This fixes a minor missing feature where get_alignment was absent from the BasicType trait).

How This Has Been Tested

I have verified the changes by running existing tests and adding a specific assertion to the test_basic_type_enum test case.

  • Ran cargo test --features "llvm20-1" (passed)
  • Local Environment: LLVM 20.1.8 (Fedora)
  • Added a check in tests/all/test_types.rs to ensure BasicTypeEnum::get_alignment returns the correct value matching the underlying type.

Checklist

Copy link
Contributor

@ErisianArchitect ErisianArchitect left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code looks good to me.

@TheDan64 TheDan64 self-requested a review December 9, 2025 18:43
@TheDan64
Copy link
Owner

TheDan64 commented Dec 9, 2025

If the method also exists on all individual variants of BasicType/Enum, we should remove them in favor of the trait impl

Copy link
Owner

@TheDan64 TheDan64 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See earlier comment

@Karesis
Copy link
Contributor Author

Karesis commented Dec 12, 2025

If the method also exists on all individual variants of BasicType/Enum, we should remove them in favor of the trait impl

I've removed the individual get_alignment implementations from specific types (IntType, FloatType, etc.) in favor of the default implementation in the BasicType trait.

@TheDan64 TheDan64 added this to the 0.8.0 milestone Dec 12, 2025
Copy link
Owner

@TheDan64 TheDan64 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@TheDan64 TheDan64 merged commit 7abf48a into TheDan64:master Dec 12, 2025
16 checks passed
@Karesis
Copy link
Contributor Author

Karesis commented Dec 12, 2025

Thanks!

:)

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.

3 participants