Core: Fix `operator[]` for typed dictionaries

core/variant/dictionary.cpp

@@ -83,35 +83,64 @@ Variant Dictionary::get_value_at_index(int p_index) const {
 	return Variant();
 }
 
+// WARNING: This operator does not validate the value type. For scripting/extensions this is
+// done in `variant_setget.cpp`. Consider using `set()` if the data might be invalid.
 Variant &Dictionary::operator[](const Variant &p_key) {
-	if (unlikely(_p->read_only)) {
-		if (likely(_p->variant_map.has(p_key))) {
-			*_p->read_only = _p->variant_map[p_key];
+	Variant key = p_key;
+	if (unlikely(!_p->typed_key.validate(key, "use `operator[]`"))) {
+		if (unlikely(!_p->typed_fallback)) {
+			_p->typed_fallback = memnew(Variant);
+		}
+		VariantInternal::initialize(_p->typed_fallback, _p->typed_value.type);
+		return *_p->typed_fallback;
+	} else if (unlikely(_p->read_only)) {
+		if (likely(_p->variant_map.has(key))) {
+			*_p->read_only = _p->variant_map[key];
 		} else {
-			*_p->read_only = Variant();
+			VariantInternal::initialize(_p->read_only, _p->typed_value.type);
 		}
-
 		return *_p->read_only;
 	} else {
-		return _p->variant_map[p_key];
+		if (unlikely(!_p->variant_map.has(key))) {
+			VariantInternal::initialize(&_p->variant_map[key], _p->typed_value.type);
+		}
+		return _p->variant_map[key];
 	}
 }
 
 const Variant &Dictionary::operator[](const Variant &p_key) const {
-	// Will not insert key, so no conversion is necessary.
-	return _p->variant_map[p_key];
+	Variant key = p_key;
+	if (unlikely(!_p->typed_key.validate(key, "use `operator[]`"))) {
+		if (unlikely(!_p->typed_fallback)) {
+			_p->typed_fallback = memnew(Variant);
+		}
+		VariantInternal::initialize(_p->typed_fallback, _p->typed_value.type);
+		return *_p->typed_fallback;
+	} else {
+		// Will not insert key, so no initialization is necessary.
+		return _p->variant_map[key];
+	}
 }
 
 const Variant *Dictionary::getptr(const Variant &p_key) const {
-	HashMap<Variant, Variant, VariantHasher, StringLikeVariantComparator>::ConstIterator E(_p->variant_map.find(p_key));
+	Variant key = p_key;
+	if (unlikely(!_p->typed_key.validate(key, "getptr"))) {
+		return nullptr;
+	}
+	HashMap<Variant, Variant, VariantHasher, StringLikeVariantComparator>::ConstIterator E(_p->variant_map.find(key));
 	if (!E) {
 		return nullptr;
 	}
 	return &E->value;
 }
 
+// WARNING: This method does not validate the value type.
 Variant *Dictionary::getptr(const Variant &p_key) {
-	HashMap<Variant, Variant, VariantHasher, StringLikeVariantComparator>::Iterator E(_p->variant_map.find(p_key));
+	Variant key = p_key;
+	if (unlikely(!_p->typed_key.validate(key, "getptr"))) {
+		return nullptr;
+	}
+	HashMap<Variant, Variant, VariantHasher, StringLikeVariantComparator>::Iterator E(_p->variant_map.find(key));
 	if (!E) {
 		return nullptr;
 	}
@@ -158,6 +187,16 @@ Variant Dictionary::get_or_add(const Variant &p_key, const Variant &p_default) {
 	return *result;
 }
 
+bool Dictionary::set(const Variant &p_key, const Variant &p_value) {
+	ERR_FAIL_COND_V_MSG(_p->read_only, false, "Dictionary is in read-only state.");
+	Variant key = p_key;
+	ERR_FAIL_COND_V(!_p->typed_key.validate(key, "set"), false);
+	Variant value = p_value;
+	ERR_FAIL_COND_V(!_p->typed_value.validate(value, "set"), false);
+	_p->variant_map[key] = value;
+	return true;
+}
+
 int Dictionary::size() const {
 	return _p->variant_map.size();
 }

core/variant/dictionary.h

@@ -59,6 +59,7 @@ public:
 	Variant get_valid(const Variant &p_key) const;
 	Variant get(const Variant &p_key, const Variant &p_default) const;
 	Variant get_or_add(const Variant &p_key, const Variant &p_default);
+	bool set(const Variant &p_key, const Variant &p_value);
 
 	int size() const;
 	bool is_empty() const;

core/variant/variant_setget.cpp

@@ -252,20 +252,7 @@ void Variant::set_named(const StringName &p_member, const Variant &p_value, bool
 		}
 	} else if (type == Variant::DICTIONARY) {
 		Dictionary &dict = *VariantGetInternalPtr<Dictionary>::get_ptr(this);
-
-		if (dict.is_read_only()) {
-			r_valid = false;
-			return;
-		}
-
-		Variant *v = dict.getptr(p_member);
-		if (v) {
-			*v = p_value;
-		} else {
-			dict[p_member] = p_value;
-		}
-
-		r_valid = true;
+		r_valid = dict.set(p_member, p_value);
 	} else {
 		r_valid = false;
 	}
@@ -721,26 +708,16 @@ struct VariantIndexedSetGet_Dictionary {
 		PtrToArg<Variant>::encode(*ptr, member);
 	}
 	static void set(Variant *base, int64_t index, const Variant *value, bool *valid, bool *oob) {
-		if (VariantGetInternalPtr<Dictionary>::get_ptr(base)->is_read_only()) {
-			*valid = false;
-			*oob = true;
-			return;
-		}
-		(*VariantGetInternalPtr<Dictionary>::get_ptr(base))[index] = *value;
-		*oob = false;
-		*valid = true;
+		*valid = VariantGetInternalPtr<Dictionary>::get_ptr(base)->set(index, *value);
+		*oob = VariantGetInternalPtr<Dictionary>::get_ptr(base)->is_read_only();
 	}
 	static void validated_set(Variant *base, int64_t index, const Variant *value, bool *oob) {
-		if (VariantGetInternalPtr<Dictionary>::get_ptr(base)->is_read_only()) {
-			*oob = true;
-			return;
-		}
-		(*VariantGetInternalPtr<Dictionary>::get_ptr(base))[index] = *value;
-		*oob = false;
+		VariantGetInternalPtr<Dictionary>::get_ptr(base)->set(index, *value);
+		*oob = VariantGetInternalPtr<Dictionary>::get_ptr(base)->is_read_only();
 	}
 	static void ptr_set(void *base, int64_t index, const void *member) {
 		Dictionary &v = *reinterpret_cast<Dictionary *>(base);
-		v[index] = PtrToArg<Variant>::convert(member);
+		v.set(index, PtrToArg<Variant>::convert(member));
 	}
 	static Variant::Type get_index_type() { return Variant::NIL; }
 	static uint32_t get_index_usage() { return PROPERTY_USAGE_DEFAULT; }
@@ -1010,16 +987,11 @@ struct VariantKeyedSetGetDictionary {
 		PtrToArg<Variant>::encode(*ptr, value);
 	}
 	static void set(Variant *base, const Variant *key, const Variant *value, bool *r_valid) {
-		if (VariantGetInternalPtr<Dictionary>::get_ptr(base)->is_read_only()) {
-			*r_valid = false;
-			return;
-		}
-		(*VariantGetInternalPtr<Dictionary>::get_ptr(base))[*key] = *value;
-		*r_valid = true;
+		*r_valid = VariantGetInternalPtr<Dictionary>::get_ptr(base)->set(*key, *value);
 	}
 	static void ptr_set(void *base, const void *key, const void *value) {
 		Dictionary &v = *reinterpret_cast<Dictionary *>(base);
-		v[PtrToArg<Variant>::convert(key)] = PtrToArg<Variant>::convert(value);
+		v.set(PtrToArg<Variant>::convert(key), PtrToArg<Variant>::convert(value));
 	}
 
 	static bool has(const Variant *base, const Variant *key, bool *r_valid) {

modules/gdscript/tests/scripts/runtime/errors/typed_dictionary_assign_differently_typed_key.gd

@@ -0,0 +1,7 @@
+func get_key() -> Variant:
+	return "key"
+
+func test():
+	var typed: Dictionary[int, int]
+	typed[get_key()] = 0
+	print('not ok')

modules/gdscript/tests/scripts/runtime/errors/typed_dictionary_assign_differently_typed_key.out

@@ -0,0 +1,6 @@
+GDTEST_RUNTIME_ERROR
+>> SCRIPT ERROR
+>> on function: test()
+>> runtime/errors/typed_dictionary_assign_differently_typed_key.gd
+>> 6
+>> Invalid assignment of property or key 'key' with value of type 'int' on a base object of type 'Dictionary[int, int]'.

modules/gdscript/tests/scripts/runtime/errors/typed_dictionary_assign_differently_typed_value.gd

@@ -0,0 +1,7 @@
+func get_value() -> Variant:
+	return "value"
+
+func test():
+	var typed: Dictionary[int, int]
+	typed[0] = get_value()
+	print("not ok")

modules/gdscript/tests/scripts/runtime/errors/typed_dictionary_assign_differently_typed_value.out

@@ -0,0 +1,6 @@
+GDTEST_RUNTIME_ERROR
+>> SCRIPT ERROR
+>> on function: test()
+>> runtime/errors/typed_dictionary_assign_differently_typed_value.gd
+>> 6
+>> Invalid assignment of property or key '0' with value of type 'String' on a base object of type 'Dictionary[int, int]'.