Support dynamic resource parameters in Rib

[afsalthaj]

Fixes #872

There were several ways to solve this problem. Here is how I approached it, and had more subtle details to handle than I expected.

• The introduction of DynamicParsedFunctionName (a concern that's only related to the Rib module) may contain Expr nodes within it. The Expr::call will now hold DynamicParsedFunctionName.

• Some parts might sound a repetition but they are not unsafe repetition, because we need to successfully form the ParsedFunctionName with the details in DynamicParsedFunctionName during Rib Instruction Execution.

• ParsedFunctionName and associated functions are untouched, and therefore the change is NOT affecting any other modules such as the worker executor.

• ParsedFunctionName implies that it's a fully formed function name, making it all type-safe.

• Rib IR is now responsible for creating the function names based on parameters. As you can imagine, it internally forms the ParsedFunctionNamewhich can be used to invoke the function in worker-service.

• The Instruction set has enough details on how to form the ParsedFunctionName safely. For example rib::ir::FunctionReferenceType::IndexedResourceMethod(String, usize, String)) has details such as resource names, how many parameters to retrieve from the stack etc.

• The resource parameters in the stack are TypeAnnotatedValue, and we convert them to wasm-wave syntax string and then create the ParsedFunctionName. T

• This may be slightly over-detailing, yet good to point out. DynamicParsedFunctionName::parse parsing is basically the mostly the same previous implementation of ParsedFunctionName::parse(str) , and obviously the difference there is resource parameters are Expr and not String. ParseFunctionName::parse simply delegates to DynamicParsedFunctionName::parse, and modules such as worker-executor is using it and by that time, the str doesn't have any dynamic parameters (or unknown identifiers). This implies the result of DynamicParsedFunctionName::parsewill have theirsExprs containing fully formed values. This can be safely converted to strusingrib::to_string(expr)which is wasm-wave` format.

• In other words, ParsedFunctionName::parse tests work as before. The only change in those text is in spaces. Example { foo: "bar" } is changed to {foo: "bar"}.

• InvokeFunction instruction is changed to first retrieve the the function name which is in stack created by CreateFunctionName instruction, before popping up the resource/method parameters from the stack and then do the actual invocation.

• Another implication of all these change is, ensuring the type inference phase is aware of these Exprs that exist in the DynamicParsedFunctionName.

• I had to also understand certain details to jump through some more hurdles and get things working, which took 1.5 more days than expected.

TODO

• More tests
• Cleanup code (as much as I could)
• Fix tests (I had to make slight changes
• Make sure backward compatible
• Add test for methods that are part of resource with zero constructor parameters. Fixed a bug
• Other clean ups, such as unnecessary requirements of Serialize and Deserialize for RibByteCode

Test cases

Testing all the functions in the cart resource in a shopping cart component

Note:

I had a different approach before (an approach where the resource parameter could be static or dynamic, contained within ParsedFunctionName, which broke many things, affected many other modules, broke many tests to later realise that the above approach is the safest and robust approach.

[afsalthaj]

Somewhere I messed up with stack interpreter

---- interpreter::rib_interpreter::interpreter_tests::test_interpreter_for_pattern_match_on_tuple_with_wild_pattern stdout ----
thread 'interpreter::rib_interpreter::interpreter_tests::test_interpreter_for_pattern_match_on_tuple_with_wild_pattern' panicked at golem-rib/src/interpreter/rib_interpreter.rs:1422:64:
called `Result::unwrap()` on an `Err` value: "Expected 10 value on the stack"
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

---- interpreter::rib_interpreter::interpreter_tests::test_interpreter_for_pattern_match_on_tuple_with_all_types stdout ----
thread 'interpreter::rib_interpreter::interpreter_tests::test_interpreter_for_pattern_match_on_tuple_with_all_types' panicked at golem-rib/src/interpreter/rib_interpreter.rs:1389:64:
called `Result::unwrap()` on an `Err` value: "Expected 10 values on the stack"


failures:
    interpreter::rib_interpreter::interpreter_tests::test_interpreter_for_pattern_match_on_tuple_with_all_types
    interpreter::rib_interpreter::interpreter_tests::test_interpreter_for_pattern_match_on_tuple_with_wild_pattern

test result: FAILED. 408 passed; 2 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.36s

[vigoo]

Isn't this a breaking change?

[afsalthaj]

how? These are grpc types only for internal persistence.

[afsalthaj]

Ok, discard my question. In terms of backward compatibility, well the function name is different now because has Exprs in it, and it's a different structure. I need to test backward compatibility.

[vigoo]

What is internal persistence? If it is stored in any database, then this is a breaking change.

[afsalthaj]

Need to test/fix backward compatibility , which is what is remaining.

[jdegoes]

Another way to handle this would be to detect breakage and recompile from source. Since the IR is not exposed to the user.

[afsalthaj]

Fixes #872

[afsalthaj]

I will push 1 more change to avoid further "expects" that I found.

[afsalthaj]

This is not required.