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

Support tuple metaclass indexers with non-literal arguments #11834

Conversation

HertzDevil
Copy link
Contributor

@HertzDevil HertzDevil commented Feb 16, 2022

Implements the following class methods:

  • Tuple.[](index : Int)
  • Tuple.[]?(index : Int)
  • NamedTuple.[](key : Symbol | String)
  • NamedTuple.[]?(key : Symbol | String)

They already work when the index is an integer literal, or when the key is a symbol or string literal. With this PR regular, non-literal variables also work.

Also "adds" a Tuple.[](range : Range) overload which always fails to compile unless range is a literal, similar to its non-metaclass counterpart.

src/named_tuple.cr Outdated Show resolved Hide resolved
src/named_tuple.cr Outdated Show resolved Hide resolved
src/tuple.cr Outdated Show resolved Hide resolved
@beta-ziliani
Copy link
Member

Thinking of #12005, to me the pattern

FIELD = :field
tuple[FIELD]

makes perfect sense; however, it won't work as expected since it will return the union of all the types of the tuple even if the value is known at compile time. How hard is it to consider this case specially?

@asterite
Copy link
Member

How hard is it to consider this case specially?

Very easy! But is there a reason to discuss this here instead of in #12005?

@beta-ziliani
Copy link
Member

No, it doesn't make sense for it to be here. (me to brain rn: GH comments are not a chat system GH comments are not a chat system GH comments are not a chat system ....)

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.

Thanks! Just one very minor concern.

src/named_tuple.cr Outdated Show resolved Hide resolved
@straight-shoota straight-shoota added this to the 1.6.0 milestone Aug 16, 2022
@straight-shoota straight-shoota merged commit e17112b into crystal-lang:master Aug 18, 2022
@HertzDevil HertzDevil deleted the feature/tuple-metaclass-indexer-var branch August 18, 2022 12:51
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