Fix forward references in ExpressionBuilder #437
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
(and update the old 2.x bits of ELM that will never work anyway).
…97-fix-forward-references-in-expressionbuilder-2 # Conflicts: # Demo/Measures.Authoring/CSharp/FHIRHelpers-4.3.000.g.cs
…nces-in-expressionbuilder
baseTwo
requested changes
Aug 1, 2024
baseTwo
previously approved these changes
Aug 1, 2024
|
I've identified a small issue where we are creating empty |
…fix-forward-references-in-expressionbuilder
…ttps://github.com/FirelyTeam/firely-cql-sdk into 397-fix-forward-references-in-expressionbuilder-2
This was
linked to
issues
Aug 2, 2024
EvanMachusak
previously approved these changes
Aug 2, 2024
ewoutkramer
dismissed
EvanMachusak’s stale review
The merge-base changed after approval.
ewoutkramer
dismissed
EvanMachusak’s stale review
The merge-base changed after approval.
EvanMachusak
previously approved these changes
Aug 5, 2024
EvanMachusak
approved these changes
Aug 5, 2024
EvanMachusak
deleted the
397-fix-forward-references-in-expressionbuilder-2
branch
|
@EvanMachusak, none of the demo projects are able to rebuild as a result of the additional commits. Could you please have a look? Issue #450 |
alexzautke
restored the
397-fix-forward-references-in-expressionbuilder-2
branch
This was referenced Aug 7, 2024
Merged
baseTwo
added a commit
that referenced
this pull request
Aug 7, 2024
|
@ewoutkramer I tried to duplicate the work into a new PR, but still getting stuck |
ewoutkramer
deleted the
397-fix-forward-references-in-expressionbuilder-2
branch
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 #397. Fixes #410.
This PR enables the translation of FunctionRef/ExpressionRef to rely on the ref's
resultTypeSpecifierinstead of having to look the definition up in the DefinitionDictionary. Not using the DefinitionDictionary avoids running into the situation where the ref is a forwards reference whereby the DefinitionDictionary does not yet contain the translated function we want to call.This PR uses the recent functionality added to LibrarySet to lookup a symbol, and expands this to be able to lookup functions by signature. Given the function name in the FUnctionRef/ExpressionRef, and, in the case of an overload, the signature, we can now lookup any function defined in any library in the LibrarySet, so we can copy over the resultTypeSpecifier.
Filling out this resultTypeSpecifier is now done in a pre-processing step, happening when we are constructing the LibrarySet. The pre-processing step will find all Refs, and then do this:
Otherwise, we cannot find the overload, and we will raise an error. This ensures that after the pre-processor has run, all Refs have a resultTypeSpecifier, and we can depend on that fact.
This PR re-uses the OverloadedFunctionDefinition, and there are some updates to the
CanCombinefunction to make sure the pre-processor fix and the CanCombine use the same criteria.I also added a bunch of useful extension methods for working with the Elm function elements and signatures.