Make all types instantiable from System.Any#1405
Conversation
|
Formatting check succeeded! |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1405 +/- ##
============================================
+ Coverage 64.38% 64.68% +0.29%
- Complexity 1921 1966 +45
============================================
Files 494 494
Lines 28020 28042 +22
Branches 5559 5569 +10
============================================
+ Hits 18042 18138 +96
+ Misses 7737 7647 -90
- Partials 2241 2257 +16 ☔ View full report in Codecov by Sentry. |
brynrhodes
left a comment
There was a problem hiding this comment.
I think this needs more investigation, I don't understand why we would need to cast the nulls in all list-valued operators as lists in order to prevent the instantiation of an interval. Is it the interplay with list promotion/demotion? Even then shouldn't the operator precedence resolve that?
|
We don't have a full-fledged solution for type unification. We detect the null as
We solve the first side first and end up with:
Then we traverse the right-side:
which is compatible and therefore type checks. Instead, what we need to do is check the left-side, realize we can't make a full inference:
And then solve the right side:
and then on the left side find the most specific type that satisfies the requirements of the right side, typically the same type.
|
|
Hi @brynrhodes @JPercival this change only affects the operators that have both list and interval overloads where A potential solution for the ambiguity problem for these expressions might be something like "implicit cast precedence"? Expressions like The last one is interesting. Currently, Ideally it should be translated as |
|
|
Closing in favor of #1428 |



This PR addresses #1401 and unblocks #1402.
Here's what happens currently when this library
is translated: The OperatorMap inside the System CompiledLibrary initially has one generic signature
(T, System.Boolean, System.String, System.String, System.String)and no concrete signatures for theMessageoperator. WhenMessage('x', true, '1', 'Message', 'This is a message')is visited by the translator, it doesn't find any matching concrete signatures at first, so it adds a new concrete signature(System.String, System.Boolean, System.String, System.String, System.String)to the OperatorMap after making sure the generic signature is instantiable from that concrete signature. This concrete signature is then used to resolve the firstMessagecall. Then, when it visits the secondMessage('y', null, '1', 'Message', 'This is a message'), it finds the existing matching concrete signature in the OperatorMap and uses that to resolve the call.Here's what happens currently when this library
fails to translate. The OperatorMap inside the System CompiledLibrary initially has no concrete signatures and one generic signature
(T, System.Boolean, System.String, System.String, System.String)for theMessageoperator. WhenMessage('y', null, '1', 'Message', 'This is a message')is visited, it doesn't find any existing matching concrete signatures (same as above). However, the generic signature is not currently instantiable from the concrete signature(System.String, System.Any, System.String, System.String, System.String)because non-any simple types are not instantiable fromSystem.Any(in this caseSystem.Booleanis not instantiable fromSystem.Any). TheMessagecall doesn't resolve because there are no matching signatures.This PR makes all types instantiable from
System.Anywhich fixes theMessagecall resolution in the above example.This change also affects the translation of expressions like
Contains(null, 'a'). TheContainsoperator has two generic signatures(List<T>, T)and(Interval<T>, T), and the expression currently translates to(List<System.Any>, System.String)when list promotion is enabled and fails to translate (Could not resolve call to operator Contains with signature (System.Any,System.String).) when list promotion is disabled. After the change, the expression fails to translate (Ambiguous generic instantiation of operator Contains between signature (list<System.Any>,System.Any) and (interval<System.Any>,System.Any).) both with and without list promotion. This is why I had to modify some existing tests to explicitly castnulls to lists or intervals to resolve the ambiguity.This change is also related to #435. Before the change,
null = nullis translated with this error:After the change, the error is:
Another issue related to this is #435. Before this change,
collapse nullfails to translate and givesCould not resolve call to operator Collapse with signature (System.Any,System.Quantity).. After the change, it translates successfully and resolves withCollapse(List<Interval<System.Any>>).Corresponding PR to update cql-tests: cqframework/cql-tests#49.