Skip to content

Commit

Permalink
Change string copy policy: only string literal are stored by pointer
Browse files Browse the repository at this point in the history
  • Loading branch information
bblanchon committed Nov 25, 2024
1 parent 1894b2d commit 127b34d
Show file tree
Hide file tree
Showing 36 changed files with 541 additions and 246 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ HEAD

* Fix support for NUL characters in `deserializeJson()`
* Make `ElementProxy` and `MemberProxy` non-copyable
* Change string copy policy: only string literal are stored by pointer

> ### BREAKING CHANGES
>
Expand Down
1 change: 0 additions & 1 deletion extras/tests/JsonArray/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ add_executable(JsonArrayTests
nesting.cpp
remove.cpp
size.cpp
std_string.cpp
subscript.cpp
unbound.cpp
)
Expand Down
151 changes: 91 additions & 60 deletions extras/tests/JsonArray/add.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,31 +17,112 @@ TEST_CASE("JsonArray::add(T)") {

SECTION("int") {
array.add(123);

REQUIRE(123 == array[0].as<int>());
REQUIRE(array[0].is<int>());
REQUIRE(array[0].is<double>());
REQUIRE(spy.log() == AllocatorLog{
Allocate(sizeofPool()),
});
}

SECTION("double") {
array.add(123.45);

REQUIRE(123.45 == array[0].as<double>());
REQUIRE(array[0].is<double>());
REQUIRE_FALSE(array[0].is<bool>());
REQUIRE(spy.log() == AllocatorLog{
Allocate(sizeofPool()),
});
}

SECTION("bool") {
array.add(true);
REQUIRE(true == array[0].as<bool>());

REQUIRE(array[0].as<bool>() == true);
REQUIRE(array[0].is<bool>());
REQUIRE_FALSE(array[0].is<int>());
REQUIRE(spy.log() == AllocatorLog{
Allocate(sizeofPool()),
});
}

SECTION("string literal") {
array.add("hello");

REQUIRE(array[0].as<std::string>() == "hello");
REQUIRE(array[0].is<const char*>());
REQUIRE(array[0].is<int>() == false);
REQUIRE(spy.log() == AllocatorLog{
Allocate(sizeofPool()),
});
}

SECTION("std::string") {
array.add("hello"_s);

REQUIRE(array[0].as<std::string>() == "hello");
REQUIRE(array[0].is<const char*>() == true);
REQUIRE(array[0].is<int>() == false);
REQUIRE(spy.log() == AllocatorLog{
Allocate(sizeofPool()),
Allocate(sizeofString("hello")),
});
}

SECTION("const char*") {
const char* str = "hello";
array.add(str);
REQUIRE(str == array[0].as<std::string>());
REQUIRE(array[0].is<const char*>());
REQUIRE_FALSE(array[0].is<int>());

REQUIRE(array[0].as<std::string>() == "hello");
REQUIRE(array[0].as<const char*>() != str);
REQUIRE(array[0].is<const char*>() == true);
REQUIRE(array[0].is<int>() == false);
REQUIRE(spy.log() == AllocatorLog{
Allocate(sizeofPool()),
Allocate(sizeofString("hello")),
});
}

SECTION("serialized(const char*)") {
array.add(serialized("{}"));

REQUIRE(doc.as<std::string>() == "[{}]");
REQUIRE(spy.log() == AllocatorLog{
Allocate(sizeofPool()),
Allocate(sizeofString("{}")),
});
}

SECTION("serialized(char*)") {
array.add(serialized(const_cast<char*>("{}")));

REQUIRE(doc.as<std::string>() == "[{}]");
REQUIRE(spy.log() == AllocatorLog{
Allocate(sizeofPool()),
Allocate(sizeofString("{}")),
});
}

SECTION("serialized(std::string)") {
array.add(serialized("{}"_s));

REQUIRE(doc.as<std::string>() == "[{}]");
REQUIRE(spy.log() == AllocatorLog{
Allocate(sizeofPool()),
Allocate(sizeofString("{}")),
});
}

SECTION("serialized(std::string)") {
array.add(serialized("\0XX"_s));

REQUIRE(doc.as<std::string>() == "[\0XX]"_s);
REQUIRE(spy.log() == AllocatorLog{
Allocate(sizeofPool()),
Allocate(sizeofString(" XX")),
});
}

#ifdef HAS_VARIABLE_LENGTH_ARRAY
Expand All @@ -52,7 +133,12 @@ TEST_CASE("JsonArray::add(T)") {

array.add(vla);

REQUIRE("world"_s == array[0]);
strcpy(vla, "hello");
REQUIRE(array[0] == "world"_s);
REQUIRE(spy.log() == AllocatorLog{
Allocate(sizeofPool()),
Allocate(sizeofString("hello")),
});
}
#endif

Expand Down Expand Up @@ -99,61 +185,6 @@ TEST_CASE("JsonArray::add(T)") {

REQUIRE(str == array[0]);
}

SECTION("should not duplicate const char*") {
array.add("world");
REQUIRE(spy.log() == AllocatorLog{
Allocate(sizeofPool()),
});
}

SECTION("should duplicate char*") {
array.add(const_cast<char*>("world"));
REQUIRE(spy.log() == AllocatorLog{
Allocate(sizeofPool()),
Allocate(sizeofString("world")),
});
}

SECTION("should duplicate std::string") {
array.add("world"_s);
REQUIRE(spy.log() == AllocatorLog{
Allocate(sizeofPool()),
Allocate(sizeofString("world")),
});
}

SECTION("should duplicate serialized(const char*)") {
array.add(serialized("{}"));
REQUIRE(spy.log() == AllocatorLog{
Allocate(sizeofPool()),
Allocate(sizeofString("{}")),
});
}

SECTION("should duplicate serialized(char*)") {
array.add(serialized(const_cast<char*>("{}")));
REQUIRE(spy.log() == AllocatorLog{
Allocate(sizeofPool()),
Allocate(sizeofString("{}")),
});
}

SECTION("should duplicate serialized(std::string)") {
array.add(serialized("{}"_s));
REQUIRE(spy.log() == AllocatorLog{
Allocate(sizeofPool()),
Allocate(sizeofString("{}")),
});
}

SECTION("should duplicate serialized(std::string)") {
array.add(serialized("\0XX"_s));
REQUIRE(spy.log() == AllocatorLog{
Allocate(sizeofPool()),
Allocate(sizeofString(" XX")),
});
}
}

TEST_CASE("JsonArray::add<T>()") {
Expand Down
34 changes: 0 additions & 34 deletions extras/tests/JsonArray/std_string.cpp

This file was deleted.

92 changes: 43 additions & 49 deletions extras/tests/JsonArray/subscript.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -59,14 +59,55 @@ TEST_CASE("JsonArray::operator[]") {
REQUIRE(false == array[0].is<int>());
}

SECTION("string literal") {
array[0] = "hello";

REQUIRE(array[0].as<std::string>() == "hello");
REQUIRE(true == array[0].is<const char*>());
REQUIRE(false == array[0].is<int>());
REQUIRE(spy.log() == AllocatorLog{
Allocate(sizeofPool()),
Allocate(sizeofString("world")),
});
}

SECTION("const char*") {
const char* str = "hello";

array[0] = str;
REQUIRE(str == array[0].as<const char*>());

REQUIRE(array[0].as<std::string>() == "hello");
REQUIRE(true == array[0].is<const char*>());
REQUIRE(false == array[0].is<int>());
REQUIRE(spy.log() == AllocatorLog{
Allocate(sizeofPool()),
Allocate(sizeofString("world")),
});
}

SECTION("std::string") {
array[0] = "hello"_s;

REQUIRE(array[0].as<std::string>() == "hello");
REQUIRE(true == array[0].is<const char*>());
REQUIRE(false == array[0].is<int>());
REQUIRE(spy.log() == AllocatorLog{
Allocate(sizeofPool()),
Allocate(sizeofString("world")),
});
}

#ifdef HAS_VARIABLE_LENGTH_ARRAY
SECTION("VLA") {
size_t i = 16;
char vla[i];
strcpy(vla, "world");

array.add("hello");
array[0] = vla;

REQUIRE(array[0] == "world"_s);
}
#endif

SECTION("nested array") {
JsonDocument doc2;
Expand Down Expand Up @@ -114,58 +155,11 @@ TEST_CASE("JsonArray::operator[]") {
REQUIRE(str == array[0]);
}

SECTION("should not duplicate const char*") {
array[0] = "world";
REQUIRE(spy.log() == AllocatorLog{
Allocate(sizeofPool()),
});
}

SECTION("should duplicate char*") {
array[0] = const_cast<char*>("world");
REQUIRE(spy.log() == AllocatorLog{
Allocate(sizeofPool()),
Allocate(sizeofString("world")),
});
}

SECTION("should duplicate std::string") {
array[0] = "world"_s;
REQUIRE(spy.log() == AllocatorLog{
Allocate(sizeofPool()),
Allocate(sizeofString("world")),
});
}

SECTION("array[0].to<JsonObject>()") {
JsonObject obj = array[0].to<JsonObject>();
REQUIRE(obj.isNull() == false);
}

#ifdef HAS_VARIABLE_LENGTH_ARRAY
SECTION("set(VLA)") {
size_t i = 16;
char vla[i];
strcpy(vla, "world");

array.add("hello");
array[0].set(vla);

REQUIRE("world"_s == array[0]);
}

SECTION("operator=(VLA)") {
size_t i = 16;
char vla[i];
strcpy(vla, "world");

array.add("hello");
array[0] = vla;

REQUIRE("world"_s == array[0]);
}
#endif

SECTION("Use a JsonVariant as index") {
array[0] = 1;
array[1] = 2;
Expand Down
Loading

0 comments on commit 127b34d

Please sign in to comment.