Add buildX methods to ITermFactory that preserve the origin of the replaced term #20
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
Apanatshka
reviewed
Jun 20, 2020
Comment on lines
296
to
311
| /** | ||
| * Copied the origin of the replaced term to the new term, unless | ||
| * the new term already has an origin of its own. | ||
| * | ||
| * This implementation mutates the new term in-place. | ||
| * | ||
| * @param newTerm the new term | ||
| * @param replacee the term being replaced | ||
| */ | ||
| private void preserveOrigins(IStrategoTerm newTerm, IStrategoTerm replacee) { | ||
| @Nullable IStrategoTerm curOrigin = OriginAttachment.getOrigin(newTerm); | ||
| if (curOrigin != null) { | ||
| // The term already has an origin of its own. | ||
| return; | ||
| } | ||
| @Nullable IStrategoTerm newOrigin = OriginAttachment.getOrigin(replacee); | ||
| if (newOrigin == null) { | ||
| // The replacee doesn't have an origin, | ||
| // so we bail out here to avoid creating a null origin attachment. | ||
| return; | ||
| } | ||
| // This mutates the term in-place. | ||
| OriginAttachment.setOrigin(newTerm, newOrigin); | ||
| } |
There was a problem hiding this comment.
Given the current type hierarchy of term factories, this isn't supposed to be here. Instead the normal TermFactory doesn't know about attachments, and the OriginTermFactory does
They where already nullable, but it was not annotated. See here, for example, where |
Virtlink
force-pushed
the
origin-preservation
branch
from
6967d91
to
51e9e9b
Compare
In StrategoTerm's constructor, this.annotations is only assigned when the argument is non-null and non-empty, implying it stays its default value of null when the argument is null or empty. The @nullable annotations explicate this fact.
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.
This PR:
buildXmethod toITermFactorythat preserve the origin of the replaced termITermFactorymethods that do not preserve origins, or have surprising and inconsistent semantics