Skip to content

Commit

Permalink
#1243: Fixed forward function overloads resolving incorrectly (#1251)
Browse files Browse the repository at this point in the history
  • Loading branch information
@brynrhodes
brynrhodes authored Oct 18, 2023
1 parent 6c80817 commit a47cfc8
Show file tree
Hide file tree
Showing 15 changed files with 550 additions and 339 deletions.
342 changes: 197 additions & 145 deletions Src/java/cql-to-elm/src/main/java/org/cqframework/cql/cql2elm/Cql2ElmVisitor.java
View file

Large diffs are not rendered by default.

75 changes: 59 additions & 16 deletions Src/java/cql-to-elm/src/main/java/org/cqframework/cql/cql2elm/LibraryBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1136,8 +1136,15 @@ public Expression resolveCall(String libraryName, String operatorName, Invocatio
return result != null ? result.getExpression() : null;
}

public Invocation resolveInvocation(String libraryName, String operatorName, Invocation invocation, boolean mustResolve, boolean allowPromotionAndDemotion, boolean allowFluent) {
Iterable<Expression> operands = invocation.getOperands();
public Invocation resolveInvocation(String libraryName, String operatorName, Invocation invocation) {
return resolveInvocation(libraryName, operatorName, invocation, true, false, false);
}

public Invocation resolveInvocation(String libraryName, String operatorName, Invocation invocation, boolean allowPromotionAndDemotion, boolean allowFluent) {
return resolveInvocation(libraryName, operatorName, invocation, true, allowPromotionAndDemotion, allowFluent);
}

public CallContext buildCallContext(String libraryName, String operatorName, Iterable<Expression> operands, boolean mustResolve, boolean allowPromotionAndDemotion, boolean allowFluent) {
List<DataType> dataTypes = new ArrayList<>();
for (Expression operand : operands) {
if (operand == null || operand.getResultType() == null) {
Expand All @@ -1148,6 +1155,12 @@ public Invocation resolveInvocation(String libraryName, String operatorName, Inv
}

CallContext callContext = new CallContext(libraryName, operatorName, allowPromotionAndDemotion, allowFluent, mustResolve, dataTypes.toArray(new DataType[dataTypes.size()]));
return callContext;
}

public Invocation resolveInvocation(String libraryName, String operatorName, Invocation invocation, boolean mustResolve, boolean allowPromotionAndDemotion, boolean allowFluent) {
Iterable<Expression> operands = invocation.getOperands();
CallContext callContext = buildCallContext(libraryName, operatorName, operands, mustResolve, allowPromotionAndDemotion, allowFluent);
OperatorResolution resolution = resolveCall(callContext);
if (resolution == null && !mustResolve) {
return null;
Expand Down Expand Up @@ -1207,6 +1220,28 @@ private Expression pruneChoices(Expression expression, DataType targetType) {
return expression;
}

public Operator resolveFunctionDefinition(FunctionDef fd) {

String libraryName = compiledLibrary.getIdentifier().getId();
String operatorName = fd.getName();
List<DataType> dataTypes = new ArrayList<>();
for (OperandDef operand : fd.getOperand()) {
if (operand == null || operand.getResultType() == null) {
throw new IllegalArgumentException(String.format("Could not determine signature for invocation of operator %s%s.",
libraryName == null ? "" : libraryName + ".", operatorName));
}
dataTypes.add(operand.getResultType());
}

CallContext callContext = new CallContext(compiledLibrary.getIdentifier().getId(), fd.getName(), false, fd.isFluent() == null ? false : fd.isFluent(), false, dataTypes.toArray(new DataType[dataTypes.size()]));
// Resolve exact, no conversion map
OperatorResolution resolution = compiledLibrary.resolveCall(callContext, null);
if (resolution != null) {
return resolution.getOperator();
}
return null;
}

public OperatorResolution resolveCall(CallContext callContext) {
OperatorResolution result = null;
if (callContext.getLibraryName() == null || callContext.getLibraryName().equals("")) {
Expand All @@ -1226,6 +1261,7 @@ public OperatorResolution resolveCall(CallContext callContext) {
}
/*
// Implicit resolution is only allowed for the system library functions.
// Except for fluent functions, so consider whether we should have an ambiguous warnings for fluent function resolution?
for (CompiledLibrary library : libraries.values()) {
OperatorResolution libraryResult = library.resolveCall(callContext, libraryBuilder.getConversionMap());
if (libraryResult != null) {
Expand Down Expand Up @@ -1278,7 +1314,7 @@ public void checkAccessLevel(String libraryName, String objectName, AccessModifi
}

public Expression resolveFunction(String libraryName, String functionName, Iterable<Expression> paramList) {
return resolveFunction(libraryName, functionName, paramList, true, false, false);
return resolveFunction(libraryName, functionName, paramList, true, false, false).getExpression();
}

private FunctionRef buildFunctionRef(String libraryName, String functionName, Iterable<Expression> paramList) {
Expand All @@ -1293,18 +1329,18 @@ private FunctionRef buildFunctionRef(String libraryName, String functionName, It
return fun;
}

public Expression resolveFunction(String libraryName, String functionName, Iterable<Expression> paramList, boolean mustResolve, boolean allowPromotionAndDemotion, boolean allowFluent) {
public Invocation resolveFunction(String libraryName, String functionName, Iterable<Expression> paramList, boolean mustResolve, boolean allowPromotionAndDemotion, boolean allowFluent) {
FunctionRef fun = buildFunctionRef(libraryName, functionName, paramList);

// First attempt to resolve as a system or local function
FunctionRefInvocation invocation = new FunctionRefInvocation(fun);
fun = (FunctionRef)resolveCall(fun.getLibraryName(), fun.getName(), invocation, mustResolve, allowPromotionAndDemotion, allowFluent);
// Attempt normal resolution, but don't require one
Invocation invocation = new FunctionRefInvocation(fun);
fun = (FunctionRef)resolveCall(fun.getLibraryName(), fun.getName(), invocation, false, allowPromotionAndDemotion, allowFluent);
if (fun != null) {
if ("System".equals(invocation.getResolution().getOperator().getLibraryName())) {
FunctionRef systemFun = buildFunctionRef(libraryName, functionName, paramList); // Rebuild the fun from the original arguments, otherwise it will resolve with conversions in place
Expression systemFunction = systemFunctionResolver.resolveSystemFunction(systemFun);
if (systemFunction != null) {
return systemFunction;
Invocation systemFunctionInvocation = systemFunctionResolver.resolveSystemFunction(systemFun);
if (systemFunctionInvocation != null) {
return systemFunctionInvocation;
}
}
else {
Expand All @@ -1320,21 +1356,25 @@ public Expression resolveFunction(String libraryName, String functionName, Itera
// 2. It is an error condition that needs to be reported
if (fun == null) {
fun = buildFunctionRef(libraryName, functionName, paramList);
invocation = new FunctionRefInvocation(fun);

if (!allowFluent) {
// Only attempt to resolve as a system function if this is not a fluent call or it is a required resolution
Expression systemFunction = systemFunctionResolver.resolveSystemFunction(fun);
Invocation systemFunction = systemFunctionResolver.resolveSystemFunction(fun);
if (systemFunction != null) {
return systemFunction;
}

checkLiteralContext();
}

fun = (FunctionRef)resolveCall(fun.getLibraryName(), fun.getName(), new FunctionRefInvocation(fun), mustResolve, allowPromotionAndDemotion, allowFluent);
fun = (FunctionRef)resolveCall(fun.getLibraryName(), fun.getName(), invocation, mustResolve, allowPromotionAndDemotion, allowFluent);
if (fun == null) {
return null;
}
}

return fun;
return invocation;
}

public void verifyComparable(DataType dataType) {
Expand Down Expand Up @@ -1629,9 +1669,9 @@ else if (conversion.getOperator() != null) {
.withName(conversion.getOperator().getName())
.withOperand(expression);

Expression systemFunction = systemFunctionResolver.resolveSystemFunction(functionRef);
if (systemFunction != null) {
return systemFunction;
Invocation systemFunctionInvocation = systemFunctionResolver.resolveSystemFunction(functionRef);
if (systemFunctionInvocation != null) {
return systemFunctionInvocation.getExpression();
}

resolveCall(functionRef.getLibraryName(), functionRef.getName(), new FunctionRefInvocation(functionRef), false, false);
Expand Down Expand Up @@ -2976,6 +3016,9 @@ private Scope getScope() {
}

public void pushExpressionContext(String context) {
if (context == null) {
throw new IllegalArgumentException("Expression context cannot be null");
}
expressionContext.push(context);
}

Expand Down
Loading

0 comments on commit a47cfc8

Please sign in to comment.