Core: Fix operator[] for typed dictionaries

core/variant/dictionary.cpp

@@ -84,22 +84,40 @@ Variant Dictionary::get_value_at_index(int p_index) const {
 }
 
 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))) {
+		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))) {
+		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 {
@@ -158,6 +176,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 @@ class Dictionary {
 	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; }

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,5 @@
+GDTEST_RUNTIME_ERROR
+>> ERROR
+>> Method/function failed. Returning: false
+>> Attempted to use a variable of type 'String' into a TypedDictionary.Key of type 'int'.
+not ok

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

@@ -0,0 +1,13 @@
+func get_value() -> Variant:
+	return "value"
+
+func test():
+	var typed: Dictionary[int, int]
+
+	# TODO: This SHOULD fail, but the assignment operator detaches the value.
+	#typed[0] = get_value()
+
+	# Will fail.
+	typed.get_or_add(0, get_value())
+
+	print("not ok")

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

@@ -0,0 +1,4 @@
+GDTEST_RUNTIME_ERROR
+>> ERROR
+>> Condition "!_p->typed_value.validate(value, "add")" is true. Returning: value
+not ok