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

Move TrieNode from kotlinx.collections #194

Merged
merged 27 commits into from
Sep 17, 2024

Conversation

oveeernight
Copy link
Contributor

No description provided.

Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

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

detekt found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.

@Saloed Saloed self-requested a review July 12, 2024 11:25
@oveeernight oveeernight force-pushed the usvm-persistent-map branch 3 times, most recently from 25d037a to 1b433cb Compare July 21, 2024 14:04
@oveeernight oveeernight force-pushed the usvm-persistent-map branch 3 times, most recently from 01d7488 to 4c8408e Compare July 24, 2024 20:15
@oveeernight oveeernight requested a review from Saloed July 24, 2024 20:47
@oveeernight oveeernight force-pushed the usvm-persistent-map branch 2 times, most recently from 05b5b85 to 476761b Compare July 24, 2024 23:04
@oveeernight oveeernight marked this pull request as draft July 25, 2024 14:22
@oveeernight oveeernight force-pushed the usvm-persistent-map branch 2 times, most recently from 2e639dc to 0209271 Compare July 25, 2024 16:08
@oveeernight oveeernight requested a review from Saloed July 25, 2024 16:21
@oveeernight oveeernight marked this pull request as ready for review July 25, 2024 16:21
@Saloed
Copy link
Collaborator

Saloed commented Jul 26, 2024

Also, it seems that we should use new (or provided from somewhere) ownership in the UComposer. Currently, we can overwrite memory with an ownership from the memory during the composition.

@oveeernight
Copy link
Contributor Author

oveeernight commented Jul 26, 2024

Also, it seems that we should use new (or provided from somewhere) ownership in the UComposer. Currently, we can overwrite memory with an ownership from the memory during the composition.

Can you clarify this? As i can see, composer stores model as memory, models are mutually shared between states, and composer.transform can modify shared memory. What else transform with default ownership can spoil?

@oveeernight oveeernight marked this pull request as draft July 26, 2024 17:35
@Saloed
Copy link
Collaborator

Saloed commented Jul 26, 2024

Also, it seems that we should use new (or provided from somewhere) ownership in the UComposer. Currently, we can overwrite memory with an ownership from the memory during the composition.

Can you clarify this? As i can see, composer stores model as memory, models are mutually shared between states, and composer.transform can modify shared memory. What else transform with default ownership can spoil?

Composer can use any memory, not only the model. So, any applyTo operation can corrupt the underlying memory

@oveeernight
Copy link
Contributor Author

oveeernight commented Jul 27, 2024

For now we use ownership of memory inside of composer. It seems to be logical, isn't it? We use default ownership only in case of model. Or probably we should provide ownership of memory that we are applying and change the ownership inside composer memory?

@oveeernight oveeernight marked this pull request as ready for review July 29, 2024 16:16
@oveeernight oveeernight requested a review from Saloed July 29, 2024 16:17
@oveeernight oveeernight marked this pull request as draft August 6, 2024 10:53
@oveeernight oveeernight requested a review from Saloed August 6, 2024 15:47
@oveeernight oveeernight marked this pull request as ready for review August 6, 2024 17:17
@Saloed Saloed merged commit 0acf657 into UnitTestBot:main Sep 17, 2024
2 checks passed
@oveeernight oveeernight deleted the usvm-persistent-map branch September 17, 2024 13:06
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.

2 participants