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

Document how to change base type of an enum #9803

Merged

Conversation

Blacksmoke16
Copy link
Member

@Blacksmoke16 Blacksmoke16 commented Oct 6, 2020

  • Add TypeNode.base_type for Enum types
  • Add Enum.base_type that uses the macro method
  • Mention the ability to change the base type in the Enum API docs

@Blacksmoke16
Copy link
Member Author

Umm, not sure why specs failed. They passed locally for me. Some caching issue?

@asterite
Copy link
Member

asterite commented Oct 6, 2020

std specs also run with the current compiler, and base_type is not defined there. You can implement it by doing typeof(values.first)

@Blacksmoke16
Copy link
Member Author

Blacksmoke16 commented Oct 6, 2020

@asterite But how are all the other macro methods useable? {% if @type.has_attribute?("Flags") %} is a TypeNode method and that doesn't cause things to break.

EDIT: NVM, I misread. I suppose, but think this macro method would be most ideal :/. Maybe I'll add a todo to update it after the next release.

@straight-shoota
Copy link
Member

Can you show a use case for this? typeof(value) already works to determine the base type. So this is essentially just adding a dedicated method for this? Where is this needed?

@Blacksmoke16
Copy link
Member Author

@straight-shoota It originally came from some chat in Gitter. https://gitter.im/crystal-lang/crystal?at=5f7afbbc6e0eb8446967c41e

I'd maybe ask @HertzDevil what he was doing it for. But IMO, there should be a better API for getting that type than doing a typeof of one of the values given it's known at compile time anyway.

@straight-shoota
Copy link
Member

Unless there's a common use case I'd say type(value) is totally fine. No need for adding an extra part to the API. typeof is evaluated at compile time, so there's no runtime impact.

@Blacksmoke16
Copy link
Member Author

Will leave it up to @HertzDevil then. If he doesn't have any specific use cases then I'm fine with closing, or at least revert the new method and keep the doc updates.

@Blacksmoke16 Blacksmoke16 changed the title Expose the base type of an enum Document how to change base type of an enum Aug 13, 2021
@Blacksmoke16
Copy link
Member Author

I kinda forgot about this, but I just reverted the methods and just made this document how to change the base type of an enum.

src/enum.cr Outdated Show resolved Hide resolved
src/enum.cr Outdated Show resolved Hide resolved
Co-authored-by: Johannes Müller <straightshoota@gmail.com>
@straight-shoota straight-shoota added this to the 1.6.0 milestone Sep 21, 2022
@straight-shoota straight-shoota merged commit 3e13d0e into crystal-lang:master Sep 26, 2022
@Blacksmoke16 Blacksmoke16 deleted the expose-enum-base-type branch September 26, 2022 12:41
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.

4 participants