Implement key-value pair iteration of dictionaries

[rodrigodias4]

Implementation includes parsing of both variables, type hints, basic static analysis and type checking, compiling, and runtime type-checking. The implementation writes an implicit assignment from the index (first for variable) of the container to the second for variable before the suite, within the compilation and code generation stages.
Added a new OPCODE (OPCODE_FOR_SECOND_VARIABLE_DICT_TYPE_TEST), used when a second variable is declared, to check if type of container is a dictionary at runtime before entering the loop.

Example grammar:
forStmt = "for" IDENTIFIER [ “:” typeHint] [ “,” IDENTIFIER [ “:” typeHint ]] "in" expression ":" stmtOrSuite ;

Unit tests added (.gd and .out):

• parser/features/for_key_value_pair : basic success test
• parser/features/for_key_value_pair_with_type_specifiers : successful key-value pair iteration on dictionary with type specifiers for both key and value variables
• analyzer/errors/for_key_value_not_dictionary : error on key-value pair iteration on non-dictionary container (static analysis at compile-time if container is statically typed)
• runtime/features/for_key_value_multiple_types : successful key-value pair iteration on dictionary with multiple types
• runtime/errors/for_key_value_not_dictionary : error on key-value pair iteration on non-dictionary container (runtime check on variant container)
• runtime/errors/for_key_value_incompatible_types : error on incompatible types between the static type hint and the container’s elements’ type

Bugsquad edit: Closes godotengine/godot-proposals#3457.

[dalexeev]

1. In my opinion, the syntax should be for key: KeyType, value: ValueType in iterable. Because a. the type is not separated from the identifier, and b. key: Type, value is less confusing than key, value: Type if you omit the value's type hint.

2. GDScript is a gradually typed programming language, so we do not require static typing anywhere.

In case the iterable value is a supertype of Dictionary (i.e. Variant), there should be no compile-time error, only the second iterator variable should be marked as unsafe.

Type checking should be performed at runtime, if necessary. If the value turns out to be a dictionary, then ok. If not, then there must be a runtime error. You can implement this either in the compiler (by generating type checking code if necessary, much like we did for match) or by adding new opcode(s) for the VM.

3. The type of the second iterator variable is not converted for implicitly convertible types (like int and float) and does not cause a runtime error for incompatible types.

This is inconsistent with the behavior of the first iterator variable.

To fix the first, you need to replace use_conversion_assign with first_use_conversion_assign and second_use_conversion_assign. See #80247 for reference.

godot/modules/gdscript/gdscript_byte_codegen.cpp

Lines 1618 to 1623 in be56cab

	if (p_use_conversion) {
	write_assign_with_conversion(p_variable, temp);
	if (p_variable.type.can_contain_object()) {
	clear_address(temp); // Can contain `RefCounted`, so clear it.
	}
	}

To fix the second, you should probably use OPCODE_ASSIGN instead of write_get().

4. Please add more tests.

Also, please remove the reference to the proposal from the commit (leave it only in the first post), because it creates unnecessary noise on each push:

[rodrigodias4]

@dalexeev

1. Changed the grammar and parser to your request, as it makes more sense:
   forStmt = "for" IDENTIFIER [ “:” typeHint] [ “,” IDENTIFIER [ “:” typeHint ]] in" expression ":" stmtOrSuite ;

2. Fixed by adding a new OPCODE (OPCODE_FOR_SECOND_VARIABLE_DICT_TYPE_TEST) that checks if the container is a dictionary at runtime before the for loop instead of checking at compile time.

3. Fixed.

4. Added tests akin to the issues you raised.