refactor: include a missing header in fs.cpp #23335

pull joankaradimov wants to merge 1 commits into bitcoin:master from joankaradimov:include-missing-header changing 1 files +1 −0
  1. joankaradimov commented at 10:38 pm on October 21, 2021: contributor

    I get this compilation error on versions 0.21.1 and 22.0:

     0fs.cpp: In member function 'bool fsbridge::FileLock::TryLock()':
     1fs.cpp:123:89: error: 'numeric_limits' is not a member of 'std'
     2  123 |     if (!LockFileEx(hFile, LOCKFILE_EXCLUSIVE_LOCK | LOCKFILE_FAIL_IMMEDIATELY, 0, std::numeric_limits<DWORD>::max(), std::numeric_limits<DWORD>::max(), &overlapped)) {
     3      |                                                                                         ^~~~~~~~~~~~~~
     4fs.cpp:123:109: error: expected primary-expression before '>' token
     5  123 |     if (!LockFileEx(hFile, LOCKFILE_EXCLUSIVE_LOCK | LOCKFILE_FAIL_IMMEDIATELY, 0, std::numeric_limits<DWORD>::max(), std::numeric_limits<DWORD>::max(), &overlapped)) {
     6      |                                                                                                             ^
     7fs.cpp:123:112: error: '::max' has not been declared; did you mean 'std::max'?
     8  123 |     if (!LockFileEx(hFile, LOCKFILE_EXCLUSIVE_LOCK | LOCKFILE_FAIL_IMMEDIATELY, 0, std::numeric_limits<DWORD>::max(), std::numeric_limits<DWORD>::max(), &overlapped)) {
     9      |                                                                                                                ^~~
    10      |                                                                                                                std::max
    11In file included from C:/dev/msys64/mingw64/include/c++/11.2.0/bits/char_traits.h:39,
    12                 from C:/dev/msys64/mingw64/include/c++/11.2.0/string:40,
    13                 from ./fs.h:9,
    14                 from fs.cpp:5:
    15C:/dev/msys64/mingw64/include/c++/11.2.0/bits/stl_algobase.h:300:5: note: 'std::max' declared here
    16  300 |     max(const _Tp& __a, const _Tp& __b, _Compare __comp)
    17      |     ^~~
    18fs.cpp:123:124: error: 'numeric_limits' is not a member of 'std'
    19  123 |     if (!LockFileEx(hFile, LOCKFILE_EXCLUSIVE_LOCK | LOCKFILE_FAIL_IMMEDIATELY, 0, std::numeric_limits<DWORD>::max(), std::numeric_limits<DWORD>::max(), &overlapped)) {
    20      |                                                                                                                            ^~~~~~~~~~~~~~
    21fs.cpp:123:144: error: expected primary-expression before '>' token
    22  123 |     if (!LockFileEx(hFile, LOCKFILE_EXCLUSIVE_LOCK | LOCKFILE_FAIL_IMMEDIATELY, 0, std::numeric_limits<DWORD>::max(), std::numeric_limits<DWORD>::max(), &overlapped)) {
    23      |                                                                                                                                                ^
    24fs.cpp:123:147: error: '::max' has not been declared; did you mean 'std::max'?
    25  123 |     if (!LockFileEx(hFile, LOCKFILE_EXCLUSIVE_LOCK | LOCKFILE_FAIL_IMMEDIATELY, 0, std::numeric_limits<DWORD>::max(), std::numeric_limits<DWORD>::max(), &overlapped)) {
    26      |                                                                                                                                                   ^~~
    27      |                                                                                                                                                   std::max
    28In file included from C:/dev/msys64/mingw64/include/c++/11.2.0/bits/char_traits.h:39,
    29                 from C:/dev/msys64/mingw64/include/c++/11.2.0/string:40,
    30                 from ./fs.h:9,
    31                 from fs.cpp:5:
    32C:/dev/msys64/mingw64/include/c++/11.2.0/bits/stl_algobase.h:300:5: note: 'std::max' declared here
    33  300 |     max(const _Tp& __a, const _Tp& __b, _Compare __comp)
    34      |     ^~~
    

    It appears that std::numeric_limits<T>::max is used without the limits header being included. Probably on other STL implementations it’s included transitively, but not in the one in MinGW. Including it fixes the compilation problem.

    Environment: OS: Windows 10 Compiler: gcc 11.2.0 Qt: 5.15.2 (included in msys2) Using the latest mingw w64 shipped with msys2.

  2. theuni commented at 11:03 pm on October 21, 2021: member

    Looks like this only affects windows because it’s in the #ifndef WIN32 #else case. So it makes sense to move this to only include this with the other windows defines.

    Trivial ACK otherwise.

  3. joankaradimov commented at 11:37 pm on October 21, 2021: contributor

    Looks like this only affects windows because it’s in the #ifndef WIN32 #else case.

    Yes, exactly. My guess is that <string> includes <limits> on Unix/gcc and <codecvt> includes <limits> on Windows/MSVC. But <codecvt> does not include <limits> on Windows/gcc. I haven’t checked that, though.

    it makes sense to move this to only include this with the other windows defines

    Well, transitive includes are implementation-specific. I’d say - just always include <limits> to be on the safe side.

  4. theuni commented at 11:53 pm on October 21, 2021: member

    Only the windows code paths use std::numeric_limits. The transitivity is moot elsewhere.

    Otherwise I’d agree :)

  5. joankaradimov commented at 0:35 am on October 22, 2021: contributor

    Oh, sorry 🤦‍♂️

    I was looking at the ifndef WIN32 at the top of the file and I completely missed the one around the implementation of FileLock::TryLock.

    Updated and force pushed.

  6. fanquake commented at 0:42 am on October 22, 2021: member
    Please write a more descriptive commit message title. i.e refactor: add missing <limits> header to fs.cpp
  7. refactor: include a missing <limits> header in fs.cpp
    ... needed for std::numeric_limits<T>::max on WIN32
    077a875d94
  8. joankaradimov commented at 1:09 am on October 22, 2021: contributor

    Please write a more descriptive commit message title. i.e refactor: add missing <limits> header to fs.cpp

    Updated and force pushed.

  9. fanquake renamed this:
    Include a missing header
    refactor: include a missing <limits> header in fs.cpp
    on Oct 22, 2021
  10. fanquake approved
  11. fanquake commented at 1:12 am on October 22, 2021: member
    ACK 077a875d94b51e3c87381133657be98989c8643e - Thanks.
  12. DrahtBot added the label Refactoring on Oct 22, 2021
  13. MarcoFalke added the label Needs backport (0.21) on Oct 22, 2021
  14. MarcoFalke added the label Needs backport (22.x) on Oct 22, 2021
  15. hebasto approved
  16. hebasto commented at 9:45 am on October 22, 2021: member

    ACK 077a875d94b51e3c87381133657be98989c8643e

    FWIW, the <limits> header is also missed in multiple other places. This is another reason to consider #15442.

  17. MarcoFalke merged this on Oct 22, 2021
  18. MarcoFalke closed this on Oct 22, 2021

  19. fanquake referenced this in commit be1a57d612 on Oct 22, 2021
  20. fanquake removed the label Needs backport (22.x) on Oct 22, 2021
  21. fanquake commented at 11:17 am on October 22, 2021: member
    Backported to 22.x in #23276.
  22. sidhujag referenced this in commit c997920cec on Oct 22, 2021
  23. joankaradimov deleted the branch on Oct 22, 2021
  24. fanquake referenced this in commit fd9d29fba1 on Feb 14, 2022
  25. fanquake referenced this in commit 282863a7e9 on Feb 15, 2022
  26. fanquake referenced this in commit 9b5f674abb on Mar 1, 2022
  27. PastaPastaPasta referenced this in commit 694264427f on Apr 3, 2022
  28. fanquake referenced this in commit cfb08c342e on Jun 9, 2022
  29. fanquake removed the label Needs backport (0.21) on Jun 9, 2022
  30. fanquake commented at 11:57 am on June 9, 2022: member
    Backported to 0.21 in #25318.
  31. fanquake referenced this in commit dca463bd81 on Jun 10, 2022
  32. delta1 referenced this in commit dcf9336784 on May 14, 2023
  33. DrahtBot locked this on Jun 9, 2023

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: 2024-12-22 12:12 UTC

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