Add runtime subtype check using static types

[SupunS]

Closes #1217

Description

This PR uses static-types for run-time subtype check in place of dynamic-types, for better performance.
The speed gains are mostly from the type-checking of collections.

Assumptions/Rationale:

• The static type for any value is known at the time of the value creation, and stays unchanged throughout the runtime (type safety).
• Since DynamicType is always a subset of StaticType, if the StaticType satisfies some condition, then the dynamic type would also satisfy the same condition.
• Thus, checking the static type to make decisions on the value should be sufficient.

To Be Done:

Optimizations:

• Directly use static types for leaf-type IsSubType/ConformsToStaticType check, instead of converting to semaType, and performing the check on semaType.

Tasks:

• Add benchmark tests and share results

---

• Targeted PR against master branch
• Linked to Github issue with discussion and accepted design OR link to spec that describes this work
• Code follows the standards mentioned here
• Updated relevant documentation
• Re-reviewed Files changed in the Github PR explorer
• Added appropriate labels

[codecov-commenter]

Codecov Report

Merging #1487 (d0484e8) into master (7cb1535) will increase coverage by 1.39%.
The diff coverage is 53.40%.

@@            Coverage Diff             @@
##           master    #1487      +/-   ##
==========================================
+ Coverage   73.01%   74.40%   +1.39%     
==========================================
  Files         288      289       +1     
  Lines       39786    55677   +15891     
==========================================
+ Hits        29048    41425   +12377     
- Misses       9263    12755    +3492     
- Partials     1475     1497      +22     

Flag	Coverage Δ
unittests	74.40% <53.40%> (+1.39%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
runtime/interpreter/statictype.go	72.87% <ø> (-0.43%)	⬇️
runtime/interpreter/value.go	63.72% <ø> (+5.67%)	⬆️
runtime/interpreter/function.go	49.27% <33.33%> (-2.01%)	⬇️
runtime/interpreter/interpreter_expression.go	84.22% <33.33%> (+0.86%)	⬆️
runtime/interpreter/simplecompositevalue.go	63.55% <50.00%> (-2.17%)	⬇️
runtime/interpreter/interpreter.go	83.71% <53.96%> (-3.06%)	⬇️
runtime/runtime.go	85.85% <80.00%> (+0.54%)	⬆️
runtime/convertValues.go	78.83% <100.00%> (+0.43%)	⬆️
runtime/interpreter/dynamictype.go	45.07% <0.00%> (-21.60%)	⬇️
runtime/ast/transaction_declaration.go	46.87% <0.00%> (-11.02%)	⬇️
... and 281 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7cb1535...d0484e8. Read the comment docs.

[github-actions]

Cadence Benchstat comparison

This branch with compared with the base branch onflow:master commit bf002e9
The command for i in {1..N}; do go test ./... -run=XXX -bench=. -shuffle=on; done was used.
Bench tests were run a total of 7 times on each branch.

Results

	old.txt	new.txt
	time/op		delta
RuntimeFungibleTokenTransfer-2	1.43ms ±37%	1.59ms ±29%	~	(p=0.456 n=7+7)
ParseInfix-2	9.35µs ± 4%	9.46µs ± 5%	~	(p=0.383 n=7+7)
ParseFungibleToken-2	219µs ± 6%	221µs ± 6%	~	(p=0.710 n=7+7)
ParseDeploy/byte_array-2	22.7ms ± 2%	22.2ms ± 6%	~	(p=0.343 n=5+7)
ParseDeploy/decode_hex-2	1.37ms ± 6%	1.34ms ± 5%	~	(p=0.383 n=7+7)
ParseArray-2	14.5ms ± 8%	14.3ms ± 8%	~	(p=0.620 n=7+7)
QualifiedIdentifierCreation/One_level-2	3.09ns ± 4%	3.20ns ± 9%	~	(p=0.175 n=7+7)
QualifiedIdentifierCreation/Three_levels-2	156ns ± 8%	163ns ± 6%	~	(p=0.165 n=7+7)
ContractInterfaceFungibleToken-2	46.4µs ± 8%	46.5µs ± 7%	~	(p=0.805 n=7+7)
CheckContractInterfaceFungibleTokenConformance-2	164µs ± 5%	166µs ± 7%	~	(p=0.535 n=7+7)
InterpretRecursionFib-2	3.05ms ± 6%	2.98ms ± 8%	~	(p=0.318 n=7+7)
NewInterpreter/new_interpreter-2	1.29µs ± 9%	1.32µs ±13%	~	(p=0.557 n=7+7)
NewInterpreter/new_sub-interpreter-2	2.43µs ± 8%	2.46µs ±11%	~	(p=0.902 n=7+7)
RuntimeResourceDictionaryValues-2	16.5ms ±12%	7.4ms ± 3%	−55.19%	(p=0.001 n=7+7)
	alloc/op		delta
NewInterpreter/new_sub-interpreter-2	1.32kB ± 0%	1.34kB ± 0%	+1.21%	(p=0.001 n=7+7)
RuntimeFungibleTokenTransfer-2	273kB ± 0%	274kB ± 0%	+0.44%	(p=0.001 n=7+6)
QualifiedIdentifierCreation/One_level-2	0.00B	0.00B	~	(all equal)
QualifiedIdentifierCreation/Three_levels-2	64.0B ± 0%	64.0B ± 0%	~	(all equal)
ContractInterfaceFungibleToken-2	26.6kB ± 0%	26.6kB ± 0%	~	(p=0.245 n=6+7)
CheckContractInterfaceFungibleTokenConformance-2	66.2kB ± 0%	66.2kB ± 0%	~	(p=0.462 n=7+7)
NewInterpreter/new_interpreter-2	848B ± 0%	848B ± 0%	~	(all equal)
InterpretRecursionFib-2	1.26MB ± 0%	1.14MB ± 0%	−9.30%	(p=0.001 n=7+7)
RuntimeResourceDictionaryValues-2	4.05MB ± 0%	2.25MB ± 0%	−44.34%	(p=0.001 n=6+7)
	allocs/op		delta
NewInterpreter/new_sub-interpreter-2	39.0 ± 0%	40.0 ± 0%	+2.56%	(p=0.001 n=7+7)
RuntimeFungibleTokenTransfer-2	4.53k ± 0%	4.58k ± 0%	+1.29%	(p=0.001 n=7+7)
QualifiedIdentifierCreation/One_level-2	0.00	0.00	~	(all equal)
QualifiedIdentifierCreation/Three_levels-2	2.00 ± 0%	2.00 ± 0%	~	(all equal)
ContractInterfaceFungibleToken-2	458 ± 0%	458 ± 0%	~	(all equal)
CheckContractInterfaceFungibleTokenConformance-2	1.07k ± 0%	1.07k ± 0%	~	(all equal)
NewInterpreter/new_interpreter-2	13.0 ± 0%	13.0 ± 0%	~	(all equal)
InterpretRecursionFib-2	26.2k ± 0%	23.8k ± 0%	−9.30%	(p=0.001 n=7+7)
RuntimeResourceDictionaryValues-2	102k ± 0%	38k ± 0%	−63.02%	(p=0.001 n=7+7)

[dsainati1]

This makes sense to me. Am I correct in understanding that this removes the need to meter dynamic types?

[SupunS]

Am I correct in understanding that this removes the need to meter dynamic types?

Yes, correct. In the long run, might be able to remove dynamic types completely.

[turbolent]

Great work here @SupunS!

Notes from our review discussion:

• The reference dynamic type contains the type of the referenced ("inner" type)

  • This is not to be confused with the borrowed type
  • As we are now using the reference static type for dynamic type checking we need to include the referenced type there
  • To get the referenced type of a storage reference value, it needs to dereference, which requires to access to storage
  • Because of this we need to pass the interpreter to Value.StaticType
  • Add test for casting storage ref to an invalid type #1243 was added as a test case before

• It is OK to pass nil to Value.StaticType if the value is not a storage reference

  • OK for run-time checks in arithmetic operations, never references
  • OK in tests

• The dynamic subtype checking function Interpreter.IsSubType was renamed to IsSubTypeOfSemaType

  • It used to use dynamic type for first argument, now uses static type
  • Most of the old dynamic subtype checking function was a "duplicate" of the sema subtype checking function and can be removed now, by delegating to the sema subtype checking function
  • However, we need to keep logic for reference types, which uses run-time information (referenced type / "inner" type, see above)
  • The reference type may be nested in an optional type, so we need to handle optionals
  • TODO: we might have to add cases for other containers (arrays, dictionaries, etc.) too

I have some suggestions / improvements for this PR which I will open in a separate PR.

Once merged, we can also discuss follow-up clean ups:

• We should be able to remove the deprecated dynamic type checking function (DeprecatedIsSubType) now, as it is not used anymore
• We should be able to remove the dynamic type hierarchy (DynamicType) now
  • However, the argument import checks importability of the value (dynamic type, not static type)
  • Value importability is mostly the same as static importability (sema.Type.IsImportable), but there are some cases where it depends on the value (e.g. for capabilities the domain matters)

[turbolent]

Opened the two follow-up PRs:

• Improve new static type-based dynamic type checking #1576
• Remove dynamic types #1577

[SupunS]

Thank you, @turbolent for the review! 🙏

TODO: we might have to add cases for other containers (arrays, dictionaries, etc.) too

I'll open a separate PR adding tests to verify this + any changes needed