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

Use enum instead of symbol for Atomic primitives #11583

Conversation

HertzDevil
Copy link
Contributor

Moves the LLVM enums to the standard library (as :nodoc: types) and relies on autocasting to transport the correct values to code generation. The casing of the enum members is important; :umax doesn't autocast to UMax, only Umax.

Out of caution, support for symbol parameters / arguments is still there and would be removed after 1.3.0; afterwards, we can use the enums directly without autocasting if we want (it seems the interpreter code might benefit from this). Atomic::Ops is not a public API so this shouldn't be considered a breaking change.

src/atomic.cr Outdated Show resolved Hide resolved
@@ -0,0 +1,27 @@
module LLVM
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any reason for extracting this particular set of enums, and not the others?

Copy link
Member

Choose a reason for hiding this comment

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

This is a partial refactor, including only the enums associated with atomic.

Copy link
Member

@beta-ziliani beta-ziliani left a comment

Choose a reason for hiding this comment

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

minor comments.

src/compiler/crystal/codegen/primitives.cr Outdated Show resolved Hide resolved
@@ -0,0 +1,27 @@
module LLVM
Copy link
Member

Choose a reason for hiding this comment

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

This is a partial refactor, including only the enums associated with atomic.

@straight-shoota straight-shoota added this to the 1.5.0 milestone May 5, 2022
@straight-shoota straight-shoota merged commit d4db6f5 into crystal-lang:master May 6, 2022
@HertzDevil HertzDevil deleted the refactor/atomic-primitives-enum branch May 8, 2022 18:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants