The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

• validation: UTXO snapshot activation #19806 (validation: UTXO snapshot activation by jamesob)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

Code review ACK 50dee5a

Thanks for the excellent explanation!

It's not important, but decltype(auto) has been available since c++14. You could mention that in the commit log if you felt like it.

Another really good reference on decltype(auto) is item 3 in Effective Modern C++ (Scott Meyers). That chapter explains one sharp edge that I wanted to test, namely that decltype(auto(i)) and decltype(auto((i))) are not always the same:

#include <iostream>
#include <mutex>

#define LOCK(cs) (void(cs))
#define WITH_LOCK(cs, code) [&]() -> decltype(auto) { LOCK(cs); code; }()

int main()
{
    std::mutex l;

    int i = WITH_LOCK(l, {int i = 1; return i;}); // this is fine, auto(decltype(i)) is an int, so this returns by value
    std::cout << "i = " << i << std::endl;

    int j = WITH_LOCK(l, {int j = 1; return (j);}); // this isn't, auto(decltype((i)) is an &int, so this returns by reference to a local variable
    std::cout << "j = " << j << std::endl; // segfault!! j is a dangling reference

    return 0;
}

This kind of bug is non-obvious from looking at the code, but it's detected by -Wreturn-local-addr, which I believe we have enabled by default:

g++ decltype_auto.cpp 
decltype_auto.cpp: In lambda function:
decltype_auto.cpp:14:46: warning: reference to local variable ‘j’ returned [-Wreturn-local-addr]
   14 |     int j = WITH_LOCK(l, {int j = 1; return (j);}); // this isn't, auto(decltype((i)) is an &int, so this returns by reference to a local variable
      |                                              ^

Is this actually clearer or less fragile (or better in some other way?) than having the explicit & and *?

I think the only benefit is when new code is written, it will less likely result in a compile error. So devs need to do the add-*-or&-at-random-places-until-it-compiles workaround less often.

Is this actually clearer or less fragile (or better in some other way?) than having the explicit & and *?

My intuition could be wrong, but I think that most people, when calling WITH_LOCK, don't expect their deducted return type to be "coerced" into an object type. In WITH_LOCK's particular case, this seem somewhat rigid and affects the ergonomics of a major use of WITH_LOCK: making use of its return value.

Pushed 50dee5a to 7a8f4b9: put some more context into the repo. Thanks jnewbery!

@jnewbery I get a different warning:

$ clang++ -std=c++17 decltype_auto.cpp
decltype_auto.cpp:14:46: warning: reference to stack memory associated with local variable 'j' returned [-Wreturn-stack-address]
    int j = WITH_LOCK(l, {int j = 1; return (j);}); // this isn't, auto(decltype((i)) is an &int, so this returns by reference to a local variable
                                             ^
decltype_auto.cpp:5:65: note: expanded from macro 'WITH_LOCK'
#define WITH_LOCK(cs, code) [&]() -> decltype(auto) { LOCK(cs); code; }()
                                                                ^~~~
1 warning generated.

And doesn't segfault.

utACK 3eb94ec

review ACK 3eb94ec