sync: Use decltype(auto) return type for WITH_LOCK

Now that we're using C++17, we can use the decltype(auto) return type
(available since C++14) for functions and lambda expressions.

As demonstrated in this commit, this can simplify cases where previously
the compiler failed to deduce the correct return type.

Just for reference, for the "assign to ref" cases fixed here, there are
3 possible solutions:

- Return a pointer and immediately deref as used before this commit
- Make sure the function/lambda returns declspec(auto) as used after
  this commit
- Class& i = WITH_LOCK(..., return std::ref(...));

-----

References:
1. https://en.cppreference.com/w/cpp/language/function#Return_type_deduction
2. https://en.cppreference.com/w/cpp/language/template_argument_deduction#Other_contexts
3. https://en.cppreference.com/w/cpp/language/auto
4. https://en.cppreference.com/w/cpp/language/decltype

Explanations:
1. https://stackoverflow.com/a/21369192
2. https://stackoverflow.com/a/21369170
3. Item 3 in Effective Modern C++ (Scott Meyers) via jnewbery

Author: dongcarl

src/sync.h

@@ -256,7 +256,22 @@ using DebugLock = UniqueLock<typename std::remove_reference<typename std::remove
 //!
 //!   int val = WITH_LOCK(cs, return shared_val);
 //!
-#define WITH_LOCK(cs, code) [&] { LOCK(cs); code; }()
+//! Note:
+//!
+//! Since the return type deduction follows that of decltype(auto), while the
+//! deduced type of:
+//!
+//!   WITH_LOCK(cs, return {int i = 1; return i;});
+//!
+//! is int, the deduced type of:
+//!
+//!   WITH_LOCK(cs, return {int j = 1; return (j);});
+//!
+//! is &int, a reference to a local variable
+//!
+//! The above is detectable at compile-time with the -Wreturn-local-addr flag in
+//! gcc and the -Wreturn-stack-address flag in clang, both enabled by default.
+#define WITH_LOCK(cs, code) [&]() -> decltype(auto) { LOCK(cs); code; }()
 
 class CSemaphore
 {

src/test/validation_chainstate_tests.cpp

@@ -35,7 +35,7 @@ BOOST_AUTO_TEST_CASE(validation_chainstate_resize_caches)
         return outp;
     };
 
-    CChainState& c1 = *WITH_LOCK(cs_main, return &manager.InitializeChainstate(mempool));
+    CChainState& c1 = WITH_LOCK(cs_main, return manager.InitializeChainstate(mempool));
     c1.InitCoinsDB(
         /* cache_size_bytes */ 1 << 23, /* in_memory */ true, /* should_wipe */ false);
     WITH_LOCK(::cs_main, c1.InitCoinsCache(1 << 23));

src/test/validation_chainstatemanager_tests.cpp

@@ -29,7 +29,7 @@ BOOST_AUTO_TEST_CASE(chainstatemanager)
 
     // Create a legacy (IBD) chainstate.
     //
-    CChainState& c1 = *WITH_LOCK(::cs_main, return &manager.InitializeChainstate(mempool));
+    CChainState& c1 = WITH_LOCK(::cs_main, return manager.InitializeChainstate(mempool));
     chainstates.push_back(&c1);
     c1.InitCoinsDB(
         /* cache_size_bytes */ 1 << 23, /* in_memory */ true, /* should_wipe */ false);
@@ -55,7 +55,7 @@ BOOST_AUTO_TEST_CASE(chainstatemanager)
 
     // Create a snapshot-based chainstate.
     //
-    CChainState& c2 = *WITH_LOCK(::cs_main, return &manager.InitializeChainstate(mempool, GetRandHash()));
+    CChainState& c2 = WITH_LOCK(::cs_main, return manager.InitializeChainstate(mempool, GetRandHash()));
     chainstates.push_back(&c2);
     c2.InitCoinsDB(
         /* cache_size_bytes */ 1 << 23, /* in_memory */ true, /* should_wipe */ false);
@@ -114,7 +114,7 @@ BOOST_AUTO_TEST_CASE(chainstatemanager_rebalance_caches)
 
     // Create a legacy (IBD) chainstate.
     //
-    CChainState& c1 = *WITH_LOCK(cs_main, return &manager.InitializeChainstate(mempool));
+    CChainState& c1 = WITH_LOCK(cs_main, return manager.InitializeChainstate(mempool));
     chainstates.push_back(&c1);
     c1.InitCoinsDB(
         /* cache_size_bytes */ 1 << 23, /* in_memory */ true, /* should_wipe */ false);
@@ -131,7 +131,7 @@ BOOST_AUTO_TEST_CASE(chainstatemanager_rebalance_caches)
 
     // Create a snapshot-based chainstate.
     //
-    CChainState& c2 = *WITH_LOCK(cs_main, return &manager.InitializeChainstate(mempool, GetRandHash()));
+    CChainState& c2 = WITH_LOCK(cs_main, return manager.InitializeChainstate(mempool, GetRandHash()));
     chainstates.push_back(&c2);
     c2.InitCoinsDB(
         /* cache_size_bytes */ 1 << 23, /* in_memory */ true, /* should_wipe */ false);