build: add Wreturn-type to Werror flags, check on more Travis machines #18145

pull Sjors wants to merge 2 commits into bitcoin:master from Sjors:2020/return-type changing 2 files +2 −1
  1. Sjors commented at 9:32 AM on February 14, 2020: member

    I overlooked a missing return false in #17577 (review) and the warning only showed up on one Travis machine (warning: control reaches end of non-void function [-Wreturn-type]).

    This PR promotes Wreturn-type to an error when configured with --enable-werror. I also added --enable-werror to the Travis machine that happened to catch this particular instance.

  2. Sjors renamed this:
    build: add Wreturn-type to Werror flags, check on more Travis machines
    WIP build: add Wreturn-type to Werror flags, check on more Travis machines
    on Feb 14, 2020
  3. Sjors commented at 9:33 AM on February 14, 2020: member

    ~(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

  4. fanquake added the label Build system on Feb 14, 2020
  5. build: add Wreturn-type to Werror flags
    This is supported by GCC and Clang.
    https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html
    https://clang.llvm.org/docs/DiagnosticsReference.html#wreturn-type
    6ba617dbe2
  6. ci: use --enable-werror on more hosts c98c26ee99
  7. Sjors force-pushed on Feb 14, 2020
  8. Sjors force-pushed on Feb 14, 2020
  9. Sjors renamed this:
    WIP build: add Wreturn-type to Werror flags, check on more Travis machines
    build: add Wreturn-type to Werror flags, check on more Travis machines
    on Feb 14, 2020
  10. vasild commented at 10:39 AM on February 14, 2020: member

    -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.

  11. vasild commented at 10:56 AM on February 14, 2020: member

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

    ACK c98c26e.

  12. practicalswift commented at 1:30 PM on February 14, 2020: contributor

    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 c98c26ee992f204b17bf17d271512b36c40ad8c5

    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.

  13. DrahtBot commented at 1:41 PM on February 14, 2020: member

    <!--e57a25ab6845829454e8d69fc972939a-->

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

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #18149 (build: change --enable-werror to enable -Werror by vasild)
    • #15112 (build: Optionally enable -Wzero-as-null-pointer-constant by Empact)
    • #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.

  14. MarcoFalke referenced this in commit 33861a8367 on Feb 16, 2020
  15. MarcoFalke merged this on Feb 16, 2020
  16. MarcoFalke closed this on Feb 16, 2020

  17. Sjors deleted the branch on Feb 16, 2020
  18. kittywhiskers referenced this in commit 1038970b69 on Jun 17, 2021
  19. kittywhiskers referenced this in commit c73d0740f1 on Jun 17, 2021
  20. kittywhiskers referenced this in commit 49921dbdd8 on Jun 17, 2021
  21. UdjinM6 referenced this in commit 988fa6a235 on Jun 23, 2021
  22. DrahtBot locked this on Feb 15, 2022

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-04-14 09:14 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me