Fix accidental mutations in Dictionary (and possibly Array and NodePath).

[Ivorforce]

Dictionary currently has a design flaw where it is possible for C++ code to mutate the underlying data even if it is supposed to be const. This PR fixes this problem by introducing SharedPtr.

In particular, it was previously possible to accidentally add key-values to const Dictionary through operator[]. This bug was triggered at least in AudioStreamWAV::load_from_buffer.

This PR is likely to cause regressions, because it exposes incorrect code. I recommend merging it soon to have time to test it before the 4.5 release.

Explanation

Some of our data structures use shared reference semantics. They currently use a pattern where the data type is merely a pointer to some shared data, and the data has a SafeRefCount to keep track of reference counts:

godot/core/variant/dictionary.h

Lines 46 to 47 in a2aefab

	class Dictionary {
	mutable DictionaryPrivate *_p;

godot/core/variant/dictionary.cpp

Lines 43 to 44 in a2aefab

	struct DictionaryPrivate {
	SafeRefCount refcount;

Unfortunately, this implementation creates a problem: const-ness of Dictionary is not transferred over to the DictionaryPrivate pointee of _p. That means all functions of Dictionary actually have full write access into the dictionary, even const ones:

const Variant &Dictionary::dummy_get(const Variant &p_variant) const {
    _p->variant_map.insert(p_variant, 2); // This, unexpectedly, does not statically fail.
}

To address this problem, DictionaryPrivate * needs to be wrapped in a smart pointer.
I decided to use this opportunity to simplify and unify implementations, and go straight for an owned smart pointer, called SharedPtr. This pointer works similarly to std::shared_ptr, i.e. all references have read-write access to it. The refcount and T contents are placed next to each other in memory, which perfectly mimicks the way it was previously set up. The new class could be used for Dictionary, Array and NodePath, alleviating house-keeping duties from them.

Affected Code

This change exposed a few minor bugs in the codebase:

• Dictionary::operator[] accidentally inserts an empty value when the key is not found, even on const access.
  • This also made it possible for typed dictionaries to contain untyped values, possibly introducing obscure bugs.
  • AudioStreamWAV::load_from_buffer is one affected caller, unknowingly modified the Dictionary parameter, even though it was const.
  • The function is supposed to crash when unknown keys are used. Instead, I made it gracefully fail. This mitigates the regressions this PR causes, and gives us time to find and regressions.
• bsearch_custom was non-const, although it could have been const.
• Functions in dictionary.cpp, array.cpp and node_path.cpp unknowingly called non-const versions of functions on data, possibly leading to worse performance (or even bugs).