Skip to content

Conversation

hvitved
Copy link
Contributor

@hvitved hvitved commented Mar 20, 2023

#11478 follow-up.

This PR introduces a new kind of ContentSet, for representing all hash/array elements where the key is of a given type (for example, arrays are elements where the key is of type int). This is purely a performance improvement, as it eliminates unnecessary fan-out.

I have deliberately not defined models-as-data tokens for these new content sets (yet), as I am not sure what would be good syntax, and I'm also not sure it will be needed in practice.

@github-actions github-actions bot added the Ruby label Mar 20, 2023
@hvitved hvitved force-pushed the ruby/element-of-type-content-set branch from a1405fc to 8d0cb8f Compare March 20, 2023 08:46
@hvitved hvitved force-pushed the ruby/element-of-type-content-set branch from 8d0cb8f to a9ef3f9 Compare March 20, 2023 09:03
@hvitved hvitved added the no-change-note-required This PR does not need a change note label Mar 20, 2023
@hvitved hvitved marked this pull request as ready for review March 20, 2023 10:45
@hvitved hvitved requested a review from a team as a code owner March 20, 2023 10:45
@hvitved hvitved requested a review from erik-krogh March 21, 2023 07:29
Copy link
Contributor

@erik-krogh erik-krogh left a comment

Choose a reason for hiding this comment

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

LGTM, but I'm not too sure about the internal DataFlow stuff.

I'm wondering which change caused the expected output in sematics.rb to change?

@hvitved
Copy link
Contributor Author

hvitved commented Mar 21, 2023

I'm wondering which change caused the expected output in sematics.rb to change?

Good question: It's the addition of this line.

@hvitved hvitved merged commit 5260d98 into github:main Mar 21, 2023
@hvitved hvitved deleted the ruby/element-of-type-content-set branch March 21, 2023 12:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-change-note-required This PR does not need a change note Ruby
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants