Skip to content

Commit d36784c

Browse files
volsavolsa
committed
resolver: Fix type-hint annotation of function pointer arguments
1 parent 77827dc commit d36784c

File tree

4 files changed

+196
-12
lines changed

4 files changed

+196
-12
lines changed

‎src/codegen/generators/expression_generator.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -976,7 +976,7 @@ impl<'ink, 'b> ExpressionCodeGenerator<'ink, 'b> {
976976
let mut variadic_parameters = Vec::new();
977977
let mut passed_param_indices = Vec::new();
978978
for (i, argument) in arguments.iter().enumerate() {
979-
let (i, parameter, _) = get_implicit_call_parameter(argument, &declared_parameters, i)?;
979+
let (i, argument, _) = get_implicit_call_parameter(argument, &declared_parameters, i)?;
980980

981981
// parameter_info includes the declaration type and type name
982982
let parameter_info = declared_parameters
@@ -1000,11 +1000,11 @@ impl<'ink, 'b> ExpressionCodeGenerator<'ink, 'b> {
10001000
.unwrap_or_else(|| {
10011001
// if we are dealing with a variadic function, we can accept all extra parameters
10021002
if pou.is_variadic() {
1003-
variadic_parameters.push(parameter);
1003+
variadic_parameters.push(argument);
10041004
Ok(None)
10051005
} else {
10061006
// we are not variadic, we have too many parameters here
1007-
Err(Diagnostic::codegen_error("Too many parameters", parameter))
1007+
Err(Diagnostic::codegen_error("Too many parameters", argument))
10081008
}
10091009
})?;
10101010

@@ -1014,11 +1014,11 @@ impl<'ink, 'b> ExpressionCodeGenerator<'ink, 'b> {
10141014
&& declaration_type.is_input())
10151015
{
10161016
let declared_parameter = declared_parameters.get(i);
1017-
self.generate_argument_by_ref(parameter, type_name, declared_parameter.copied())?
1017+
self.generate_argument_by_ref(argument, type_name, declared_parameter.copied())?
10181018
} else {
10191019
// by val
1020-
if !parameter.is_empty_statement() {
1021-
self.generate_expression(parameter)?
1020+
if !argument.is_empty_statement() {
1021+
self.generate_expression(argument)?
10221022
} else if let Some(param) = declared_parameters.get(i) {
10231023
self.generate_empty_expression(param)?
10241024
} else {

‎src/resolver.rs

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -307,7 +307,23 @@ impl TypeAnnotator<'_> {
307307

308308
let mut generics_candidates: FxHashMap<String, Vec<String>> = FxHashMap::default();
309309
let mut params = vec![];
310-
let mut parameters = parameters.into_iter();
310+
let mut parameters = if self.annotation_map.get(operator).is_some_and(|opt| opt.is_fnptr()) {
311+
// When dealing with a function pointer (which are only supported in the context of methods), the
312+
// first argument will be a instance of the POU, e.g. `fnPtrToMyFbEcho^(instanceFb)`, hence we
313+
// must skip the first argument as otherwise the remainig arguments will receive an incorrect type
314+
// hint. Again, for example assume we have `fnPtrToMyFbEcho^(instanceFb, 'stringValue', 5)` and
315+
// we do not skip the first argument. Then, `instanceFB` will have a type-hint of "STRING" and
316+
// `stringValue` will have a type-hint on `DINT`. This then results in an error in the codegen.
317+
// This is super hacky and ugly imo, but because function pointers are internal only constructs I
318+
// think it should be fine (kinda). In a perfect world we would desugar method calls such as
319+
// `instanceFb.echo('stringValue', 5)` into `echo(instanceFb, 'stringValue', 5)` and update the
320+
// declared parameters to also include the instance parameter. As a positive side-effect it would
321+
// result in us not distinguishing between functions and methods in the codegen (though that
322+
// currently is not a big deal) but also you not reading this comment :^)
323+
parameters[1..].iter()
324+
} else {
325+
parameters.iter()
326+
};
311327

312328
// If we are dealing with an action call statement, we need to get the declared parameters from the parent POU in order
313329
// to annotate them with the correct type hint.
@@ -1027,6 +1043,7 @@ impl AnnotationMapImpl {
10271043
}
10281044

10291045
pub fn annotate_type_hint(&mut self, s: &AstNode, annotation: StatementAnnotation) {
1046+
dbg!(&s, &annotation);
10301047
self.type_hint_map.insert(s.get_id(), annotation);
10311048
}
10321049

@@ -2203,6 +2220,7 @@ impl<'i> TypeAnnotator<'i> {
22032220
}
22042221

22052222
fn visit_call_statement(&mut self, statement: &AstNode, ctx: &VisitorContext) {
2223+
dbg!(&statement);
22062224
let (operator, parameters_stmt) = if let AstStatement::CallStatement(data, ..) = statement.get_stmt()
22072225
{
22082226
(data.operator.as_ref(), data.parameters.as_deref())

‎src/resolver/tests/fnptr.rs

Lines changed: 169 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,11 @@ fn function_pointer_method_with_no_arguments() {
2020
2121
FUNCTION main
2222
VAR
23+
instanceFb: Fb;
2324
echoPtr: POINTER TO Fb.echo := ADR(Fb.echo);
2425
END_VAR
2526
26-
echoPtr^();
27+
echoPtr^(instanceFb);
2728
END_FUNCTION
2829
",
2930
ids.clone(),
@@ -81,7 +82,7 @@ fn function_pointer_method_with_arguments() {
8182
instanceFb: Fb;
8283
END_VAR
8384
84-
echoPtr^();
85+
echoPtr^(instanceFb);
8586
echoPtr^(instanceFb, localIn, localOut, localInOut);
8687
echoPtr^(instanceFb, in := localIn, out => localOut, inout := localInOut);
8788
END_FUNCTION
@@ -423,3 +424,169 @@ fn void_pointer_casting() {
423424
insta::assert_debug_snapshot!(annotations.get_hint(&call.operator), @"None");
424425
}
425426
}
427+
428+
#[test]
429+
fn function_pointer_arguments_have_correct_type_hint() {
430+
let id_provider = IdProvider::default();
431+
let (unit, index) = index_with_ids(
432+
r"
433+
FUNCTION_BLOCK A
434+
METHOD printArgs
435+
VAR_INPUT
436+
message: STRING;
437+
value: DINT;
438+
END_VAR
439+
END_METHOD
440+
END_FUNCTION_BLOCK
441+
442+
FUNCTION main
443+
VAR
444+
instanceA: A;
445+
printArgs: POINTER TO A.printArgs := ADR(A.printArgs);
446+
END_VAR
447+
448+
instanceA.printArgs('value =', 5);
449+
printArgs^(instanceA, 'value =', 5);
450+
END_FUNCTION
451+
",
452+
id_provider.clone(),
453+
);
454+
455+
let (annotations, ..) = TypeAnnotator::visit_unit(&index, &unit, id_provider);
456+
457+
// instanceA.printArgs('value =', 5);
458+
// ^^^^^^^^^
459+
{
460+
let node = &unit.implementations.iter().find(|imp| imp.name == "main").unwrap().statements[0];
461+
462+
let AstStatement::CallStatement(CallStatement { parameters: Some(parameters), .. }) = &node.stmt
463+
else {
464+
unreachable!();
465+
};
466+
467+
let AstStatement::ExpressionList(arguments) = &parameters.stmt else {
468+
unreachable!();
469+
};
470+
471+
insta::assert_debug_snapshot!(&arguments[0], @r#"
472+
LiteralString {
473+
value: "value =",
474+
is_wide: false,
475+
}
476+
"#);
477+
478+
insta::assert_debug_snapshot!(annotations.get(&arguments[0]), @r#"
479+
Some(
480+
Value {
481+
resulting_type: "__STRING_7",
482+
},
483+
)
484+
"#);
485+
486+
insta::assert_debug_snapshot!(annotations.get_type(&arguments[0], &index), @r#"
487+
Some(
488+
DataType {
489+
name: "__STRING_7",
490+
initial_value: None,
491+
information: String {
492+
size: LiteralInteger(
493+
8,
494+
),
495+
encoding: Utf8,
496+
},
497+
nature: String,
498+
location: SourceLocation {
499+
span: None,
500+
},
501+
},
502+
)
503+
"#);
504+
505+
insta::assert_debug_snapshot!(annotations.get_type_hint(&arguments[0], &index), @r#"
506+
Some(
507+
DataType {
508+
name: "STRING",
509+
initial_value: None,
510+
information: String {
511+
size: LiteralInteger(
512+
81,
513+
),
514+
encoding: Utf8,
515+
},
516+
nature: String,
517+
location: SourceLocation {
518+
span: None,
519+
},
520+
},
521+
)
522+
"#);
523+
}
524+
525+
// printArgs^(instanceA, 'value =', 5);
526+
// ^^^^^^^^^
527+
{
528+
let node = &unit.implementations.iter().find(|imp| imp.name == "main").unwrap().statements[1];
529+
530+
let AstStatement::CallStatement(CallStatement { parameters: Some(parameters), .. }) = &node.stmt
531+
else {
532+
unreachable!();
533+
};
534+
535+
let AstStatement::ExpressionList(arguments) = &parameters.stmt else {
536+
unreachable!();
537+
};
538+
539+
insta::assert_debug_snapshot!(&arguments[1], @r#"
540+
LiteralString {
541+
value: "value =",
542+
is_wide: false,
543+
}
544+
"#);
545+
546+
insta::assert_debug_snapshot!(annotations.get(&arguments[1]), @r#"
547+
Some(
548+
Value {
549+
resulting_type: "__STRING_7",
550+
},
551+
)
552+
"#);
553+
554+
insta::assert_debug_snapshot!(annotations.get_type(&arguments[1], &index), @r#"
555+
Some(
556+
DataType {
557+
name: "__STRING_7",
558+
initial_value: None,
559+
information: String {
560+
size: LiteralInteger(
561+
8,
562+
),
563+
encoding: Utf8,
564+
},
565+
nature: String,
566+
location: SourceLocation {
567+
span: None,
568+
},
569+
},
570+
)
571+
"#);
572+
573+
insta::assert_debug_snapshot!(annotations.get_type_hint(&arguments[1], &index), @r#"
574+
Some(
575+
DataType {
576+
name: "STRING",
577+
initial_value: None,
578+
information: String {
579+
size: LiteralInteger(
580+
81,
581+
),
582+
encoding: Utf8,
583+
},
584+
nature: String,
585+
location: SourceLocation {
586+
span: None,
587+
},
588+
},
589+
)
590+
"#);
591+
}
592+
}

‎tests/lit/single/polymorphism/fnptr/method_call_by_void_pointer_cast.st

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,7 @@ FUNCTION main
3131
// table is also accessed in desugared code (though the access happens in the POU rather than a local
3232
// variable in the main function)
3333
// CHECK: value = 5
34+
// CHECK: value = 5
35+
FnTable#(fnTableRef^).printArgs^(instanceA, 'value =', 5);
3436
FnTable#(fnTableRef^).printArgs^(instanceA, message := 'value =', value := 5);
35-
36-
// FIXME: Issues when not having named arguments?
37-
// FnTable#(fnTableRef^).printArgs^(instanceA, 'value =', 5);
3837
END_FUNCTION

0 commit comments

Comments
 (0)