Merge #25092: doc: various developer notes updates

6542842 Add clang lifetimebound section to developer notes (Jon Atack) e66b321 Add C++ functions and methods section to developer notes (Jon Atack) 5fca70f Link in developer notes style to internal interface exception (Jon Atack) fc4cb85 Prefer Python for scripts in developer notes (Jon Atack) 370120e Remove obsolete BDB ENABLE_WALLET section in developer notes (Jon Atack) Pull request description: A few updates noticed while working on a lifetimebound section. - Remove obsolete BDB ENABLE_WALLET section (only one file, src/wallet/bdb.h, still has a `db_cxx.h` BDB header) - Prefer Python for scripts in developer notes (and a few miscellaneous touch-ups) - In the code style section, add a link to the internal interface exception so that people are aware of it - Add a "C++ functions and methods" section - Add a Clang `lifetimebound` attribute section ACKs for top commit: laanwj: ACK 6542842 jarolrod: code review ACK 6542842 Tree-SHA512: d2ae335cac899451d5c7bdc8e94fd82053a5f44ac1ba444bdde71abaaa24a519713c1078f3a8f07ec8466b181788a613fd3c68061e54b3fdc8cd6f3e3f9791ec
bitcoin · May 16, 2022 · bc2eee7 · bc2eee7
2 parents b74a6dd + 6542842
commit bc2eee7
Showing 1 changed file with 47 additions and 24 deletions.
diff --git a/doc/developer-notes.md b/doc/developer-notes.md
@@ -32,6 +32,7 @@ Developer Notes
     - [C++ data structures](#c-data-structures)
     - [Strings and formatting](#strings-and-formatting)
     - [Shadowing](#shadowing)
+    - [Lifetimebound](#lifetimebound)
     - [Threads and synchronization](#threads-and-synchronization)
     - [Scripts](#scripts)
         - [Shebang](#shebang)
@@ -96,7 +97,10 @@ code.
     Guidelines](https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Renum-caps),
     which recommend using `snake_case`.  Please use what seems appropriate.
   - Class names, function names, and method names are UpperCamelCase
-    (PascalCase). Do not prefix class names with `C`.
+    (PascalCase). Do not prefix class names with `C`. See [Internal interface
+    naming style](#internal-interface-naming-style) for an exception to this
+    convention.
+
   - Test suite naming convention: The Boost test suite in file
     `src/test/foo_tests.cpp` should be named `foo_tests`. Test suite names
     must be unique.
@@ -138,6 +142,27 @@ public:
 } // namespace foo
 ```
 
+Coding Style (C++ functions and methods)
+--------------------
+
+- When ordering function parameters, place input parameters first, then any
+  in-out parameters, followed by any output parameters.
+
+- *Rationale*: API consistency.
+
+- Prefer returning values directly to using in-out or output parameters. Use
+  `std::optional` where helpful for returning values.
+
+- *Rationale*: Less error-prone (no need for assumptions about what the output
+  is initialized to on failure), easier to read, and often the same or better
+  performance.
+
+- Generally, use `std::optional` to represent optional by-value inputs (and
+  instead of a magic default value, if there is no real default). Non-optional
+  input parameters should usually be values or const references, while
+  non-optional in-out and output parameters should usually be references, as
+  they cannot be null.
+
 Coding Style (C++ named arguments)
 ------------------------------
 
@@ -657,10 +682,6 @@ Wallet
 
 - Make sure that no crashes happen with run-time option `-disablewallet`.
 
-- Include `db_cxx.h` (BerkeleyDB header) only when `ENABLE_WALLET` is set.
-
-  - *Rationale*: Otherwise compilation of the disable-wallet build will fail in environments without BerkeleyDB.
-
 General C++
 -------------
 
@@ -863,12 +884,22 @@ from using a different variable with the same name),
 please name variables so that their names do not shadow variables defined in the source code.
 
 When using nested cycles, do not name the inner cycle variable the same as in
-the upper cycle, etc.
+the outer cycle, etc.
+
+Lifetimebound
+--------------
+
+The [Clang `lifetimebound`
+attribute](https://clang.llvm.org/docs/AttributeReference.html#lifetimebound)
+can be used to tell the compiler that a lifetime is bound to an object and
+potentially see a compile-time warning if the object has a shorter lifetime from
+the invalid use of a temporary. You can use the attribute by adding a `LIFETIMEBOUND`
+annotation defined in `src/attributes.h`; please grep the codebase for examples.
 
 Threads and synchronization
 ----------------------------
 
-- Prefer `Mutex` type to `RecursiveMutex` one
+- Prefer `Mutex` type to `RecursiveMutex` one.
 
 - Consistently use [Clang Thread Safety Analysis](https://clang.llvm.org/docs/ThreadSafetyAnalysis.html) annotations to
   get compile-time warnings about potential race conditions in code. Combine annotations in function declarations with
@@ -947,6 +978,8 @@ TRY_LOCK(cs_vNodes, lockNodes);
 Scripts
 --------------------------
 
+Write scripts in Python rather than bash, when possible.
+
 ### Shebang
 
 - Use `#!/usr/bin/env bash` instead of obsolete `#!/bin/bash`.
@@ -1389,22 +1422,9 @@ communication:
   virtual boost::signals2::scoped_connection connectTipChanged(TipChangedFn fn) = 0;
   ```
 
-- For consistency and friendliness to code generation tools, interface method
-  input and inout parameters should be ordered first and output parameters
-  should come last.
+- Interface methods should not be overloaded.
 
-  Example:
-
-  ```c++
-  // Good: error output param is last
-  virtual bool broadcastTransaction(const CTransactionRef& tx, CAmount max_fee, std::string& error) = 0;
-
-  // Bad: error output param is between input params
-  virtual bool broadcastTransaction(const CTransactionRef& tx, std::string& error, CAmount max_fee) = 0;
-  ```
-
-- For friendliness to code generation tools, interface methods should not be
-  overloaded:
+  *Rationale*: consistency and friendliness to code generation tools.
 
   Example:
 
@@ -1418,10 +1438,13 @@ communication:
   virtual bool disconnect(NodeId id) = 0;
   ```
 
-- For consistency and friendliness to code generation tools, interface method
-  names should be `lowerCamelCase` and standalone function names should be
+### Internal interface naming style
+
+- Interface method names should be `lowerCamelCase` and standalone function names should be
   `UpperCamelCase`.
 
+  *Rationale*: consistency and friendliness to code generation tools.
+
   Examples:
 
   ```c++