search

gulrak / filesystem

An implementation of C++17 std::filesystem for C++11 /C++14/C++17/C++20 on Windows, macOS, Linux and FreeBSD.
MIT License
1.31k stars 168 forks source link

clang-tidy-18 reports `The value 'XYZ' provided to the cast expression is not in the valid range of values for 'copy_options'` #183

Closed bbannier closed 2 weeks ago

bbannier commented 2 weeks ago

We are trying to bump CI linting to clang-tidy-18 and notice that it reports an issue around fs::copy, namely that code is triggered which attempts to cast an integer to an enum type which does not contain the value.

We are building with C++17, but I can see this by adding a test to the project also (C++11).

TEST_CASE("foo")
{
    {
        TemporaryDirectory t(TempOpt::change_path);
        std::error_code ec;
        generateFile("file");
        REQUIRE(fs::exists("file"));

        CHECK_NOTHROW(fs::copy("file", "file2")); // Just trigger a `copy`.
        CHECK(!ec);

        CHECK(fs::exists("file2"));
        CHECK(fs::symlink_status("file").type() == fs::file_type::regular);
    }
}
$ clang-tidy foo.cpp
3 warnings generated.
/tmp/spicy/3rdparty/filesystem/include/ghc/filesystem.hpp:1434:12: warning: The value '160' provided to the cast expression is not in the valid range of values for 'copy_options' [clang-analyzer-optin.core.EnumCastOutOfRange]
 1434 |     return static_cast<Enum>(static_cast<underlying>(X) | static_cast<underlying>(Y));
      |            ^
/tmp/spicy/3rdparty/filesystem/include/ghc/filesystem.hpp:748:12: note: enum declared here
  748 | enum class copy_options : uint16_t {
      | ~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~
  749 |     none = 0,
      |     ~~~~~~~~~
  750 | 
  751 |     skip_existing = 1,
      |     ~~~~~~~~~~~~~~~~~~
  752 |     overwrite_existing = 2,
      |     ~~~~~~~~~~~~~~~~~~~~~~~
  753 |     update_existing = 4,
      |     ~~~~~~~~~~~~~~~~~~~~
  754 | 
  755 |     recursive = 8,
      |     ~~~~~~~~~~~~~~
  756 | 
  757 |     copy_symlinks = 0x10,
      |     ~~~~~~~~~~~~~~~~~~~~~
  758 |     skip_symlinks = 0x20,
      |     ~~~~~~~~~~~~~~~~~~~~~
  759 | 
  760 |     directories_only = 0x40,
      |     ~~~~~~~~~~~~~~~~~~~~~~~~
  761 |     create_symlinks = 0x80,
      |     ~~~~~~~~~~~~~~~~~~~~~~~
  762 | #ifndef GHC_OS_WEB
      | ~~~~~~~~~~~~~~~~~~
  763 |     create_hard_links = 0x100
      |     ~~~~~~~~~~~~~~~~~~~~~~~~~
/tmp/spicy/3rdparty/filesystem/test/multi1.cpp:101:9: note: Left side of '&&' is false
  101 |         REQUIRE(fs::exists("file"));
      |         ^
/tmp/spicy/3rdparty/filesystem/test/catch.hpp:17659:24: note: expanded from macro 'REQUIRE'
 17659 | #define REQUIRE( ... ) INTERNAL_CATCH_TEST( "REQUIRE", Catch::ResultDisposition::Normal, __VA_ARGS__  )
       |                        ^
/tmp/spicy/3rdparty/filesystem/test/catch.hpp:2712:31: note: expanded from macro 'INTERNAL_CATCH_TEST'
 2712 |     } while( (void)0, (false) && static_cast<bool>( !!(__VA_ARGS__) ) )
      |                               ^
/tmp/spicy/3rdparty/filesystem/test/multi1.cpp:101:9: note: Loop condition is false.  Exiting loop
  101 |         REQUIRE(fs::exists("file"));
      |         ^
/tmp/spicy/3rdparty/filesystem/test/catch.hpp:17659:24: note: expanded from macro 'REQUIRE'
 17659 | #define REQUIRE( ... ) INTERNAL_CATCH_TEST( "REQUIRE", Catch::ResultDisposition::Normal, __VA_ARGS__  )
       |                        ^
/tmp/spicy/3rdparty/filesystem/test/catch.hpp:2702:5: note: expanded from macro 'INTERNAL_CATCH_TEST'
 2702 |     do { \
      |     ^
/tmp/spicy/3rdparty/filesystem/test/multi1.cpp:103:23: note: Calling 'copy'
  103 |         CHECK_NOTHROW(fs::copy("file", "file2"));
      |                       ^
/tmp/spicy/3rdparty/filesystem/test/catch.hpp:17682:117: note: expanded from macro 'CHECK_NOTHROW'
 17682 | #define CHECK_NOTHROW( ... ) INTERNAL_CATCH_NO_THROW( "CHECK_NOTHROW", Catch::ResultDisposition::ContinueOnFailure, __VA_ARGS__ )
       |                                                                                                                     ^~~~~~~~~~~
/tmp/spicy/3rdparty/filesystem/test/catch.hpp:2729:31: note: expanded from macro 'INTERNAL_CATCH_NO_THROW'
 2729 |             static_cast<void>(__VA_ARGS__); \
      |                               ^~~~~~~~~~~
/tmp/spicy/3rdparty/filesystem/include/ghc/filesystem.hpp:3794:5: note: Calling 'copy'
 3794 |     copy(from, to, copy_options::none);
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/tmp/spicy/3rdparty/filesystem/include/ghc/filesystem.hpp:3800:5: note: Calling 'copy'
 3800 |     copy(from, to, options, ec);
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~
/tmp/spicy/3rdparty/filesystem/include/ghc/filesystem.hpp:3817:5: note: Taking false branch
 3817 |     if ((options & (copy_options::skip_symlinks | copy_options::copy_symlinks | copy_options::create_symlinks)) != copy_options::none) {
      |     ^
/tmp/spicy/3rdparty/filesystem/include/ghc/filesystem.hpp:3823:5: note: Taking false branch
 3823 |     if (!exists(fs_from)) {
      |     ^
/tmp/spicy/3rdparty/filesystem/include/ghc/filesystem.hpp:3829:21: note: Calling 'operator|<ghc::filesystem::copy_options>'
 3829 |     if ((options & (copy_options::skip_symlinks | copy_options::create_symlinks)) != copy_options::none) {
      |                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/tmp/spicy/3rdparty/filesystem/include/ghc/filesystem.hpp:1434:12: note: The value '160' provided to the cast expression is not in the valid range of values for 'copy_options'
 1434 |     return static_cast<Enum>(static_cast<underlying>(X) | static_cast<underlying>(Y));
      |            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/tmp/spicy/3rdparty/filesystem/include/ghc/filesystem.hpp:1434:12: warning: The value '176' provided to the cast expression is not in the valid range of values for 'copy_options' [clang-analyzer-optin.core.EnumCastOutOfRange]
 1434 |     return static_cast<Enum>(static_cast<underlying>(X) | static_cast<underlying>(Y));
      |            ^
/tmp/spicy/3rdparty/filesystem/include/ghc/filesystem.hpp:748:12: note: enum declared here
  748 | enum class copy_options : uint16_t {
      | ~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~
  749 |     none = 0,
      |     ~~~~~~~~~
  750 | 
  751 |     skip_existing = 1,
      |     ~~~~~~~~~~~~~~~~~~
  752 |     overwrite_existing = 2,
      |     ~~~~~~~~~~~~~~~~~~~~~~~
  753 |     update_existing = 4,
      |     ~~~~~~~~~~~~~~~~~~~~
  754 | 
  755 |     recursive = 8,
      |     ~~~~~~~~~~~~~~
  756 | 
  757 |     copy_symlinks = 0x10,
      |     ~~~~~~~~~~~~~~~~~~~~~
  758 |     skip_symlinks = 0x20,
      |     ~~~~~~~~~~~~~~~~~~~~~
  759 | 
  760 |     directories_only = 0x40,
      |     ~~~~~~~~~~~~~~~~~~~~~~~~
  761 |     create_symlinks = 0x80,
      |     ~~~~~~~~~~~~~~~~~~~~~~~
  762 | #ifndef GHC_OS_WEB
      | ~~~~~~~~~~~~~~~~~~
  763 |     create_hard_links = 0x100
      |     ~~~~~~~~~~~~~~~~~~~~~~~~~
/tmp/spicy/3rdparty/filesystem/test/multi1.cpp:101:9: note: Left side of '&&' is false
  101 |         REQUIRE(fs::exists("file"));
      |         ^
/tmp/spicy/3rdparty/filesystem/test/catch.hpp:17659:24: note: expanded from macro 'REQUIRE'
 17659 | #define REQUIRE( ... ) INTERNAL_CATCH_TEST( "REQUIRE", Catch::ResultDisposition::Normal, __VA_ARGS__  )
       |                        ^
/tmp/spicy/3rdparty/filesystem/test/catch.hpp:2712:31: note: expanded from macro 'INTERNAL_CATCH_TEST'
 2712 |     } while( (void)0, (false) && static_cast<bool>( !!(__VA_ARGS__) ) )
      |                               ^
/tmp/spicy/3rdparty/filesystem/test/multi1.cpp:101:9: note: Loop condition is false.  Exiting loop
  101 |         REQUIRE(fs::exists("file"));
      |         ^
/tmp/spicy/3rdparty/filesystem/test/catch.hpp:17659:24: note: expanded from macro 'REQUIRE'
 17659 | #define REQUIRE( ... ) INTERNAL_CATCH_TEST( "REQUIRE", Catch::ResultDisposition::Normal, __VA_ARGS__  )
       |                        ^
/tmp/spicy/3rdparty/filesystem/test/catch.hpp:2702:5: note: expanded from macro 'INTERNAL_CATCH_TEST'
 2702 |     do { \
      |     ^
/tmp/spicy/3rdparty/filesystem/test/multi1.cpp:103:23: note: Calling 'copy'
  103 |         CHECK_NOTHROW(fs::copy("file", "file2"));
      |                       ^
/tmp/spicy/3rdparty/filesystem/test/catch.hpp:17682:117: note: expanded from macro 'CHECK_NOTHROW'
 17682 | #define CHECK_NOTHROW( ... ) INTERNAL_CATCH_NO_THROW( "CHECK_NOTHROW", Catch::ResultDisposition::ContinueOnFailure, __VA_ARGS__ )
       |                                                                                                                     ^~~~~~~~~~~
/tmp/spicy/3rdparty/filesystem/test/catch.hpp:2729:31: note: expanded from macro 'INTERNAL_CATCH_NO_THROW'
 2729 |             static_cast<void>(__VA_ARGS__); \
      |                               ^~~~~~~~~~~
/tmp/spicy/3rdparty/filesystem/include/ghc/filesystem.hpp:3794:5: note: Calling 'copy'
 3794 |     copy(from, to, copy_options::none);
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/tmp/spicy/3rdparty/filesystem/include/ghc/filesystem.hpp:3800:5: note: Calling 'copy'
 3800 |     copy(from, to, options, ec);
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~
/tmp/spicy/3rdparty/filesystem/include/ghc/filesystem.hpp:3817:21: note: Calling 'operator|<ghc::filesystem::copy_options>'
 3817 |     if ((options & (copy_options::skip_symlinks | copy_options::copy_symlinks | copy_options::create_symlinks)) != copy_options::none) {
      |                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/tmp/spicy/3rdparty/filesystem/include/ghc/filesystem.hpp:1434:12: note: The value '176' provided to the cast expression is not in the valid range of values for 'copy_options'
 1434 |     return static_cast<Enum>(static_cast<underlying>(X) | static_cast<underlying>(Y));
      |            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/tmp/spicy/3rdparty/filesystem/include/ghc/filesystem.hpp:1434:12: warning: The value '48' provided to the cast expression is not in the valid range of values for 'copy_options' [clang-analyzer-optin.core.EnumCastOutOfRange]
 1434 |     return static_cast<Enum>(static_cast<underlying>(X) | static_cast<underlying>(Y));
      |            ^
/tmp/spicy/3rdparty/filesystem/include/ghc/filesystem.hpp:748:12: note: enum declared here
  748 | enum class copy_options : uint16_t {
      | ~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~
  749 |     none = 0,
      |     ~~~~~~~~~
  750 | 
  751 |     skip_existing = 1,
      |     ~~~~~~~~~~~~~~~~~~
  752 |     overwrite_existing = 2,
      |     ~~~~~~~~~~~~~~~~~~~~~~~
  753 |     update_existing = 4,
      |     ~~~~~~~~~~~~~~~~~~~~
  754 | 
  755 |     recursive = 8,
      |     ~~~~~~~~~~~~~~
  756 | 
  757 |     copy_symlinks = 0x10,
      |     ~~~~~~~~~~~~~~~~~~~~~
  758 |     skip_symlinks = 0x20,
      |     ~~~~~~~~~~~~~~~~~~~~~
  759 | 
  760 |     directories_only = 0x40,
      |     ~~~~~~~~~~~~~~~~~~~~~~~~
  761 |     create_symlinks = 0x80,
      |     ~~~~~~~~~~~~~~~~~~~~~~~
  762 | #ifndef GHC_OS_WEB
      | ~~~~~~~~~~~~~~~~~~
  763 |     create_hard_links = 0x100
      |     ~~~~~~~~~~~~~~~~~~~~~~~~~
/tmp/spicy/3rdparty/filesystem/test/multi1.cpp:101:9: note: Left side of '&&' is false
  101 |         REQUIRE(fs::exists("file"));
      |         ^
/tmp/spicy/3rdparty/filesystem/test/catch.hpp:17659:24: note: expanded from macro 'REQUIRE'
 17659 | #define REQUIRE( ... ) INTERNAL_CATCH_TEST( "REQUIRE", Catch::ResultDisposition::Normal, __VA_ARGS__  )
       |                        ^
/tmp/spicy/3rdparty/filesystem/test/catch.hpp:2712:31: note: expanded from macro 'INTERNAL_CATCH_TEST'
 2712 |     } while( (void)0, (false) && static_cast<bool>( !!(__VA_ARGS__) ) )
      |                               ^
/tmp/spicy/3rdparty/filesystem/test/multi1.cpp:101:9: note: Loop condition is false.  Exiting loop
  101 |         REQUIRE(fs::exists("file"));
      |         ^
/tmp/spicy/3rdparty/filesystem/test/catch.hpp:17659:24: note: expanded from macro 'REQUIRE'
 17659 | #define REQUIRE( ... ) INTERNAL_CATCH_TEST( "REQUIRE", Catch::ResultDisposition::Normal, __VA_ARGS__  )
       |                        ^
/tmp/spicy/3rdparty/filesystem/test/catch.hpp:2702:5: note: expanded from macro 'INTERNAL_CATCH_TEST'
 2702 |     do { \
      |     ^
/tmp/spicy/3rdparty/filesystem/test/multi1.cpp:103:23: note: Calling 'copy'
  103 |         CHECK_NOTHROW(fs::copy("file", "file2"));
      |                       ^
/tmp/spicy/3rdparty/filesystem/test/catch.hpp:17682:117: note: expanded from macro 'CHECK_NOTHROW'
 17682 | #define CHECK_NOTHROW( ... ) INTERNAL_CATCH_NO_THROW( "CHECK_NOTHROW", Catch::ResultDisposition::ContinueOnFailure, __VA_ARGS__ )
       |                                                                                                                     ^~~~~~~~~~~
/tmp/spicy/3rdparty/filesystem/test/catch.hpp:2729:31: note: expanded from macro 'INTERNAL_CATCH_NO_THROW'
 2729 |             static_cast<void>(__VA_ARGS__); \
      |                               ^~~~~~~~~~~
/tmp/spicy/3rdparty/filesystem/include/ghc/filesystem.hpp:3794:5: note: Calling 'copy'
 3794 |     copy(from, to, copy_options::none);
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/tmp/spicy/3rdparty/filesystem/include/ghc/filesystem.hpp:3800:5: note: Calling 'copy'
 3800 |     copy(from, to, options, ec);
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~
/tmp/spicy/3rdparty/filesystem/include/ghc/filesystem.hpp:3817:21: note: Calling 'operator|<ghc::filesystem::copy_options>'
 3817 |     if ((options & (copy_options::skip_symlinks | copy_options::copy_symlinks | copy_options::create_symlinks)) != copy_options::none) {
      |                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/tmp/spicy/3rdparty/filesystem/include/ghc/filesystem.hpp:1434:12: note: The value '48' provided to the cast expression is not in the valid range of values for 'copy_options'
 1434 |     return static_cast<Enum>(static_cast<underlying>(X) | static_cast<underlying>(Y));
      |            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
$ clang-tidy --version
Homebrew LLVM version 18.1.7
  Optimized build.

TBH, I am not entirely sure how this cast comes about at all, please let me know if I am holding it wrong (hi Steffen!).

gulrak commented 2 weeks ago

Hi Benjamin! :smile:

The issue is, that the standard demands copy_options to be a class enum of bit values like this (the actual values of each flag are unspecified though):

enum class copy_options : uint16_t {
    none = 0,
    skip_existing = 1,
    overwrite_existing = 2,
    update_existing = 4,
    recursive = 8,
    copy_symlinks = 0x10,
    skip_symlinks = 0x20,
    directories_only = 0x40,
    create_symlinks = 0x80,
    create_hard_links = 0x100
};

And to have operators in place to use them in and/or operations as bit-mask creating result values of type copy_options.

It is clear that any result of these operators will not be a value of the enum, but that is how the API is specified, so I don't see a good way to fix this without either breaking compliance or filling the enum up with dummy values for all valid combinations (that would be 512 entries in total). Neither seems like a good idea and for example libstdc++ does nearly the same.

bbannier commented 2 weeks ago

Thanks for having a look and the explanations! I believe I see similar issues around fs::perm_options and it seems a more general issues. Looking at llvm/llvm-project#76208 this seems to be a false positive, so I am closing this issue. I do not think it makes much sense to suppress this since clang-tidy seems to plan to fix this.