From ecde3b5992f567de0618e4329ef56bcd1babc945 Mon Sep 17 00:00:00 2001 From: Aldo Canepa Date: Fri, 6 Mar 2026 13:36:13 -0800 Subject: [PATCH 1/3] Remove initializer_list section --- cpp.md | 34 ++++------------------------------ 1 file changed, 4 insertions(+), 30 deletions(-) diff --git a/cpp.md b/cpp.md index 20dd252..e8958eb 100644 --- a/cpp.md +++ b/cpp.md @@ -396,33 +396,7 @@ These cases are easy to spot when declaring variables as `const` because the con **Related PR:** [Example implementation](https://github.com/Expensify/Auth/pull/16628/files) -### 2. Avoid Initializer List Constructor for Large Objects - -The `JSON::Value(initializer_list)` constructor always creates copies due to a limitation in C++ when using `initializer_list`, which inherently involves copying. This is not an issue specific to the `JSON::Value` implementation. For large objects (transactions, reportActions, reports, personalDetails, participants, etc.), use assignment with `move()` instead to avoid unnecessary copies. - -#### Examples - -**❌ Bad: Forces copies** -```cpp -return JSON::Value({ - {"structuredReportsForOnyx", move(structuredReportsForOnyx)}, - {"structuredReportsNVPsForOnyx", move(structuredReportsNVPsForOnyx)}, - {"lastMessageData", move(lastMessageData)} -}); -``` - -**✅ Good: Avoids copies** -```cpp -JSON::Value returnValue(JSON::OBJECT); -returnValue["structuredReportsForOnyx"] = move(structuredReportsForOnyx); -returnValue["structuredReportsNVPsForOnyx"] = move(structuredReportsNVPsForOnyx); -returnValue["lastMessageData"] = move(lastMessageData); -return returnValue; -``` - -**Related PR:** [Example implementation](https://github.com/Expensify/Auth/pull/14365) - -### 3. Use Move Semantics and Helper Functions +### 2. Use Move Semantics and Helper Functions When data is no longer needed after an operation, use `move()` to transfer ownership and avoid copies. Combine with `JSON::Value` helper functions for cleaner code. @@ -482,7 +456,7 @@ Value& operator=(Value&& v); **Related PR:** [Example implementation](https://github.com/Expensify/Auth/pull/14820) -### 4. Handle Ternary Operations with References Carefully +### 3. Handle Ternary Operations with References Carefully Assigning ternary operator results to references can cause copies if one side creates a temporary object. @@ -503,7 +477,7 @@ const JSON::Value& customLists = policy.hasMember("customLists") ? policy["custo const JSON::Value& customLists = policy.getMemberWithDefault("customLists", JSON::Utils::EMPTY_ARRAY); ``` -### 5. Optimize Functions Returning by Value +### 4. Optimize Functions Returning by Value Functions returning by value always create copies, even when assigned to reference variables. @@ -552,7 +526,7 @@ With these overloads, the dangerous code above will call the correct overload an **Related PR:** [Example implementation](https://github.com/Expensify/Auth/pull/15772/files#r2157875973) (see overloads added in JSON::Value) -### 6. Optimize Transformer Functions +### 5. Optimize Transformer Functions These are functions that take parameters, transform the data, and return it in a different shape. From 385b4cc6a575d505e8a33f6edcf5843d91ce1ad3 Mon Sep 17 00:00:00 2001 From: Aldo Canepa Date: Fri, 6 Mar 2026 14:17:21 -0800 Subject: [PATCH 2/3] Simplify example - not necessary to avoid initializer_list using helper function --- cpp.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/cpp.md b/cpp.md index e8958eb..9d6bd99 100644 --- a/cpp.md +++ b/cpp.md @@ -342,7 +342,7 @@ This section outlines common patterns that cause unnecessary JSON copying in C++ ### 1. Use References for Read-Only Operations -When declaring a `JSON::Value` as `const`, it cannot be modified, so we should always use references to avoid unnecessary copies. +When declaring a `JSON::Value` as `const`, it cannot be modified, so we should always use references to avoid unnecessary copies when iterating or passing it into a function. #### Examples @@ -396,15 +396,15 @@ These cases are easy to spot when declaring variables as `const` because the con **Related PR:** [Example implementation](https://github.com/Expensify/Auth/pull/16628/files) -### 2. Use Move Semantics and Helper Functions +### 2. Use Move Semantics and move-enabled Functions -When data is no longer needed after an operation, use `move()` to transfer ownership and avoid copies. Combine with `JSON::Value` helper functions for cleaner code. +When data is no longer needed after an operation, use `move()` to transfer ownership and avoid copies: #### Examples The following example assumes `participants` is not used after creating and queueing the onyx updates. -**❌ Bad: Multiple unnecessary copies** +**❌ Bad: Unnecessary copy of participants** ```cpp AuthCommand::currentCommand->queueOnyxUpdates( @@ -429,7 +429,7 @@ AuthCommand::currentCommand->queueOnyxUpdates( OnyxUpdates::createOnyxUpdate( OnyxUpdates::METHOD_MERGE, OnyxUpdates::COLLECTION_REPORT + to_string(reportID), - JSON::Value::singleEntryObject("participants", move(participants)) + JSON::Value(map{{"participants", move(participants)}}) ) ) ); From 0c890bbc4295db3a57ef6e1861d7668504b65ed3 Mon Sep 17 00:00:00 2001 From: Aldo Canepa Date: Fri, 6 Mar 2026 14:52:14 -0800 Subject: [PATCH 3/3] Revert some changes --- cpp.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/cpp.md b/cpp.md index 9d6bd99..1a3fdb6 100644 --- a/cpp.md +++ b/cpp.md @@ -396,15 +396,15 @@ These cases are easy to spot when declaring variables as `const` because the con **Related PR:** [Example implementation](https://github.com/Expensify/Auth/pull/16628/files) -### 2. Use Move Semantics and move-enabled Functions +### 2. Use Move Semantics and Helper Functions -When data is no longer needed after an operation, use `move()` to transfer ownership and avoid copies: +When data is no longer needed after an operation, use `move()` to transfer ownership and avoid copies. Combine with `JSON::Value` helper functions for cleaner code. #### Examples The following example assumes `participants` is not used after creating and queueing the onyx updates. -**❌ Bad: Unnecessary copy of participants** +**❌ Bad: Multiple unnecessary copies** ```cpp AuthCommand::currentCommand->queueOnyxUpdates( @@ -429,7 +429,7 @@ AuthCommand::currentCommand->queueOnyxUpdates( OnyxUpdates::createOnyxUpdate( OnyxUpdates::METHOD_MERGE, OnyxUpdates::COLLECTION_REPORT + to_string(reportID), - JSON::Value(map{{"participants", move(participants)}}) + JSON::Value::singleEntryObject("participants", move(participants)) ) ) );