(don't merge this yet, I need to drop the last commit 4294b2f which should produce the error)

Proof that Travis indeed catches this: https://travis-ci.org/bitcoin/bitcoin/jobs/650359000#L2713

-Wreturn-type is enabled by default (even without -Wall) in clang and gcc:

https://releases.llvm.org/9.0.0/tools/clang/docs/DiagnosticsReference.html#wreturn-type
https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html

The following code:

enum Color {
    RED,
    BLUE
};

bool isWarm(Color c) {
    switch (c) {
    case RED:
        return true;
    case BLUE:
        return false;
    }
}

bool isCold(Color c) {
    if (c == BLUE) {
        return true;
    }
}

produces:

Clang 7.0.1, 8.0.1, 9.0.1 with no options, with just -Wall, with -Wall -Wreturn-type: only a warning for isCold(): warning: control may reach end of non-void function [-Wreturn-type].

GCC 9.2.0 with no options, with just -Wall, with -Wall -Wreturn-type: a warning for both functions: warning: control reaches end of non-void function [-Wreturn-type].

Clang is staying silent about isWarm() because it sees that all enumeration values are handled in the switch. If I remove the case BLUE:, then I get two warnings for isWarm():

warning: enumeration value 'BLUE' not handled in switch [-Wswitch]
warning: control may reach end of non-void function [-Wreturn-type]

That said, adding explicitly -Wreturn-type is unnecessary because it is enabled by default.

Ah! But we are using -Werror=whatever which selectively turns only some warnings into errors. Sorry for the noise above.

ACK c98c26e.

Thanks for working on this! I think we're currently somewhat underutilising the help we can get from compilers in the form of compiler diagnostics to prevent mistakes and this PR brings us in the right direction :)

ACK c98c26e

If you have time please consider submitting a PR enabling the subset of compiler diagnostics discussed in #17344 that you think make sense. I would be glad to review such PRs, so don't hesitate to ping me in for review.

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

• build: add --enable-isystem and change --enable-werror to enable -Werror #18149 (build: change --enable-werror to enable -Werror by vasild)
• build: Optionally enable -Wzero-as-null-pointer-constant #15112 (build: Optionally enable -Wzero-as-null-pointer-constant by Empact)
• Build: enable -Wdocumentation via isystem #14920 (Build: enable -Wdocumentation via isystem by Empact)

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.