Resolve dupe apply to mixed members #2030
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fixes smithy-lang#2004. When an apply statement targets a mixed in member, the trait application is stored until the mixed in member has been synthesized. However, if you load the same model file multiple times in the same loader, the trait application is only stored once, and removed when it is claimed, so when de-conflicting the duplicate mixed-in member definitions, only one of them will have the trait applied, resulting in a conflict. This commit updates the loader to keep track of when these unclaimed traits have been claimed, rather than just removing them when claimed. As long as the traits have been claimed once, we know they aren't being applied to a non-existent shape. This allows the traits to be claimed multiple times if necessary. Test cases were added for two different situations: 1. If the exact same model is loaded multiple times, so there are multiple instances where the traits need to be claimed. In this case there shouldn't be conflicts like in the issue. 2. If the apply statement is loaded multiple times, but the member it targets is only loaded once. In this case there shouldn't be any unclaimed traits, even though there are technically multiple of the same apply statements.
haydenbaker
approved these changes
Nov 6, 2023
mtdowling
approved these changes
Nov 8, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #2004.
When an apply statement targets a mixed in member, the trait application is stored until the mixed in member has been synthesized. However, if you load the same model file multiple times in the same loader, the trait application is only stored once, and removed when it is claimed, so when de-conflicting the duplicate mixed-in member definitions, only one of them will have the trait applied, resulting in a conflict.
This commit updates the loader to keep track of when these unclaimed traits have been claimed, rather than just removing them when claimed. As long as the traits have been claimed once, we know they aren't being applied to a non-existent shape. This allows the traits to be claimed multiple times if necessary.
Test cases were added for two different situations:
statements.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.