Clang 14 emits -Wunreachable-code warnings #29334

1. 
   hebasto commented at 9:22 pm on January 27, 2024: member

   0% clang --version
   1Apple clang version 14.0.0 (clang-1400.0.29.202)
   2Target: x86_64-apple-darwin21.6.0
   3Thread model: posix
   4InstalledDir: /Library/Developer/CommandLineTools/usr/bin
   

    0...
    1CXX      wallet/libbitcoin_wallet_a-walletdb.o
    2wallet/walletdb.cpp:1518:15: warning: code will never be executed [-Wunreachable-code]
    3        error = Untranslated(strprintf("Failed to open database path '%s'. Build does not support Berkeley DB database format.", fs::PathToString(path)));
    4              ^
    5wallet/walletdb.cpp:1513:19: note: silence by adding parentheses to mark code as explicitly dead
    6    if constexpr (true) {
    7                  ^
    8                  /* DISABLES CODE */ ( )
    9wallet/walletdb.cpp:1506:19: warning: code will never be executed [-Wunreachable-code]
   10            error = Untranslated(strprintf("Failed to open database path '%s'. Build does not support SQLite database format.", fs::PathToString(path)));
   11                  ^
   12wallet/walletdb.cpp:1501:23: note: silence by adding parentheses to mark code as explicitly dead
   13        if constexpr (true) {
   14                      ^
   15                      /* DISABLES CODE */ ( )
   162 warnings generated.
   17...
   

   Seems related to #29315.

   Not happening with Clang 15 in the macOS CI job.

2. 
   maflcko commented at 12:16 pm on January 28, 2024: member

   It is sometimes possible for older compilers to print false positive warnings.

   This warning can safely be ignored.

3. maflcko added the label Build system on Jan 28, 2024
4. maflcko added the label Questions and Help on Jan 28, 2024
5. 
   hebasto commented at 12:26 pm on January 28, 2024: member

   It is sometimes possible for older compilers to print false positive warnings.

   I agree.

   This warning can safely be ignored.

   It might require some efforts if building routines involve -Werror.

6. maflcko removed the label Questions and Help on Jan 28, 2024
7. maflcko added the label Brainstorming on Jan 28, 2024
8. 
   fanquake commented at 10:00 am on January 29, 2024: member

   It might require some efforts if building routines involve -Werror.

   Wouldn’t you just pass -Wno-unreachable-code ?

   In any case, I don’t think there’s anything else we can do here, other than suggest using a fixed compiler and/or stop using -Werror.

9. 
   fanquake commented at 11:38 am on January 29, 2024: member

   Closing as upgrade compiler / pass a flag to disable the false positive / don’t pass error flags to turn known false positives into compile failures.
10. fanquake closed this on Jan 29, 2024

    ---

11. 
    vasild commented at 11:02 am on February 16, 2024: contributor

    The code in question can be improved:

    0#ifdef USE_BDB
    1    if constexpr (true) {
    2        return MakeBerkeleyDatabase(path, options, status, error);
    3    } else
    4#endif
    5    {   
    6        error = Untranslated(strprintf("Failed to open database path '%s'. Build does not support Berkeley DB database format.", fs::PathToString(path)));
    7        status = DatabaseStatus::FAILED_BAD_FORMAT;
    8        return nullptr;
    9    }
    

    If USE_BDB is defined then this becomes:

    0    if constexpr (true) {
    1        return MakeBerkeleyDatabase(path, options, status, error);
    2    } else
    3    {   
    4        error = Untranslated(strprintf("Failed to open database path '%s'. Build does not support Berkeley DB database format.", fs::PathToString(path)));
    5        status = DatabaseStatus::FAILED_BAD_FORMAT;
    6        return nullptr;
    7    }
    

    and rightfully there is a warning that the else branch will never be reached. I do not know why other compilers don’t print the warning. Anyway the following is equivalent, shorter, easier to read and warning-free:

    0#ifdef USE_BDB
    1    return MakeBerkeleyDatabase(path, options, status, error);
    2#else
    3    error = Untranslated(strprintf("Failed to open database path '%s'. Build does not support Berkeley DB database format.", fs::PathToString(path)));
    4    status = DatabaseStatus::FAILED_BAD_FORMAT;
    5    return nullptr;
    6#endif
    

12. 
    maflcko commented at 11:08 am on February 16, 2024: member

    Sure, but that is simply a revert of the commit mentioned in the description. Thus, the code would no longer be compiled unconditionally, and compile errors may be missed.
13. 
    vasild commented at 2:33 pm on February 16, 2024: contributor

    I see, commented in #29315 (comment)

Contributors
hebasto maflcko fanquake vasild

Labels
Brainstorming Build system