refactor: include a missing <limits> header in fs.cpp by joankaradimov · Pull Request #23335 · bitcoin/bitcoin

joankaradimov · 2021-10-21T22:38:59Z

I get this compilation error on versions 0.21.1 and 22.0:

fs.cpp: In member function 'bool fsbridge::FileLock::TryLock()':
fs.cpp:123:89: error: 'numeric_limits' is not a member of 'std'
  123 |     if (!LockFileEx(hFile, LOCKFILE_EXCLUSIVE_LOCK | LOCKFILE_FAIL_IMMEDIATELY, 0, std::numeric_limits<DWORD>::max(), std::numeric_limits<DWORD>::max(), &overlapped)) {
      |                                                                                         ^~~~~~~~~~~~~~
fs.cpp:123:109: error: expected primary-expression before '>' token
  123 |     if (!LockFileEx(hFile, LOCKFILE_EXCLUSIVE_LOCK | LOCKFILE_FAIL_IMMEDIATELY, 0, std::numeric_limits<DWORD>::max(), std::numeric_limits<DWORD>::max(), &overlapped)) {
      |                                                                                                             ^
fs.cpp:123:112: error: '::max' has not been declared; did you mean 'std::max'?
  123 |     if (!LockFileEx(hFile, LOCKFILE_EXCLUSIVE_LOCK | LOCKFILE_FAIL_IMMEDIATELY, 0, std::numeric_limits<DWORD>::max(), std::numeric_limits<DWORD>::max(), &overlapped)) {
      |                                                                                                                ^~~
      |                                                                                                                std::max
In file included from C:/dev/msys64/mingw64/include/c++/11.2.0/bits/char_traits.h:39,
                 from C:/dev/msys64/mingw64/include/c++/11.2.0/string:40,
                 from ./fs.h:9,
                 from fs.cpp:5:
C:/dev/msys64/mingw64/include/c++/11.2.0/bits/stl_algobase.h:300:5: note: 'std::max' declared here
  300 |     max(const _Tp& __a, const _Tp& __b, _Compare __comp)
      |     ^~~
fs.cpp:123:124: error: 'numeric_limits' is not a member of 'std'
  123 |     if (!LockFileEx(hFile, LOCKFILE_EXCLUSIVE_LOCK | LOCKFILE_FAIL_IMMEDIATELY, 0, std::numeric_limits<DWORD>::max(), std::numeric_limits<DWORD>::max(), &overlapped)) {
      |                                                                                                                            ^~~~~~~~~~~~~~
fs.cpp:123:144: error: expected primary-expression before '>' token
  123 |     if (!LockFileEx(hFile, LOCKFILE_EXCLUSIVE_LOCK | LOCKFILE_FAIL_IMMEDIATELY, 0, std::numeric_limits<DWORD>::max(), std::numeric_limits<DWORD>::max(), &overlapped)) {
      |                                                                                                                                                ^
fs.cpp:123:147: error: '::max' has not been declared; did you mean 'std::max'?
  123 |     if (!LockFileEx(hFile, LOCKFILE_EXCLUSIVE_LOCK | LOCKFILE_FAIL_IMMEDIATELY, 0, std::numeric_limits<DWORD>::max(), std::numeric_limits<DWORD>::max(), &overlapped)) {
      |                                                                                                                                                   ^~~
      |                                                                                                                                                   std::max
In file included from C:/dev/msys64/mingw64/include/c++/11.2.0/bits/char_traits.h:39,
                 from C:/dev/msys64/mingw64/include/c++/11.2.0/string:40,
                 from ./fs.h:9,
                 from fs.cpp:5:
C:/dev/msys64/mingw64/include/c++/11.2.0/bits/stl_algobase.h:300:5: note: 'std::max' declared here
  300 |     max(const _Tp& __a, const _Tp& __b, _Compare __comp)
      |     ^~~

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.

theuni · 2021-10-21T23:03:31Z

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.

joankaradimov · 2021-10-21T23:37:12Z

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.

theuni · 2021-10-21T23:53:24Z

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

Otherwise I'd agree :)

joankaradimov · 2021-10-22T00:35:47Z

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.

fanquake · 2021-10-22T00:42:02Z

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

... needed for std::numeric_limits<T>::max on WIN32

joankaradimov · 2021-10-22T01:09:22Z

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

Updated and force pushed.

fanquake

ACK 077a875 - Thanks.

hebasto

ACK 077a875

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

... needed for std::numeric_limits<T>::max on WIN32 Github-Pull: bitcoin#23335 Rebased-From: 077a875

fanquake · 2021-10-22T11:17:48Z

Backported to 22.x in #23276.

…s.cpp 077a875 refactor: include a missing <limits> header in fs.cpp (Joan Karadimov) Pull request description: I get this compilation error on versions `0.21.1` and `22.0`: ``` fs.cpp: In member function 'bool fsbridge::FileLock::TryLock()': fs.cpp:123:89: error: 'numeric_limits' is not a member of 'std' 123 | if (!LockFileEx(hFile, LOCKFILE_EXCLUSIVE_LOCK | LOCKFILE_FAIL_IMMEDIATELY, 0, std::numeric_limits<DWORD>::max(), std::numeric_limits<DWORD>::max(), &overlapped)) { | ^~~~~~~~~~~~~~ fs.cpp:123:109: error: expected primary-expression before '>' token 123 | if (!LockFileEx(hFile, LOCKFILE_EXCLUSIVE_LOCK | LOCKFILE_FAIL_IMMEDIATELY, 0, std::numeric_limits<DWORD>::max(), std::numeric_limits<DWORD>::max(), &overlapped)) { | ^ fs.cpp:123:112: error: '::max' has not been declared; did you mean 'std::max'? 123 | if (!LockFileEx(hFile, LOCKFILE_EXCLUSIVE_LOCK | LOCKFILE_FAIL_IMMEDIATELY, 0, std::numeric_limits<DWORD>::max(), std::numeric_limits<DWORD>::max(), &overlapped)) { | ^~~ | std::max In file included from C:/dev/msys64/mingw64/include/c++/11.2.0/bits/char_traits.h:39, from C:/dev/msys64/mingw64/include/c++/11.2.0/string:40, from ./fs.h:9, from fs.cpp:5: C:/dev/msys64/mingw64/include/c++/11.2.0/bits/stl_algobase.h:300:5: note: 'std::max' declared here 300 | max(const _Tp& __a, const _Tp& __b, _Compare __comp) | ^~~ fs.cpp:123:124: error: 'numeric_limits' is not a member of 'std' 123 | if (!LockFileEx(hFile, LOCKFILE_EXCLUSIVE_LOCK | LOCKFILE_FAIL_IMMEDIATELY, 0, std::numeric_limits<DWORD>::max(), std::numeric_limits<DWORD>::max(), &overlapped)) { | ^~~~~~~~~~~~~~ fs.cpp:123:144: error: expected primary-expression before '>' token 123 | if (!LockFileEx(hFile, LOCKFILE_EXCLUSIVE_LOCK | LOCKFILE_FAIL_IMMEDIATELY, 0, std::numeric_limits<DWORD>::max(), std::numeric_limits<DWORD>::max(), &overlapped)) { | ^ fs.cpp:123:147: error: '::max' has not been declared; did you mean 'std::max'? 123 | if (!LockFileEx(hFile, LOCKFILE_EXCLUSIVE_LOCK | LOCKFILE_FAIL_IMMEDIATELY, 0, std::numeric_limits<DWORD>::max(), std::numeric_limits<DWORD>::max(), &overlapped)) { | ^~~ | std::max In file included from C:/dev/msys64/mingw64/include/c++/11.2.0/bits/char_traits.h:39, from C:/dev/msys64/mingw64/include/c++/11.2.0/string:40, from ./fs.h:9, from fs.cpp:5: C:/dev/msys64/mingw64/include/c++/11.2.0/bits/stl_algobase.h:300:5: note: 'std::max' declared here 300 | max(const _Tp& __a, const _Tp& __b, _Compare __comp) | ^~~ ``` 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. ACKs for top commit: fanquake: ACK 077a875 - Thanks. hebasto: ACK 077a875 Tree-SHA512: 2289cb72fa3a28470f4250833be66079482d1392189b1e4679330dad109a8ae67b1d6d51cb635dbc1947c00c6c4cfbc4d7c9ae02269b932cfa1475583adaf73d

... needed for std::numeric_limits<T>::max on WIN32 Github-Pull: bitcoin#23335 Rebased-From: 077a875

269553f test: Call ceildiv helper with integer (Martin Zumsande) 2f60fc6 ci: Replace soon EOL hirsute with jammy (MarcoFalke) 801b0f0 build: patch qt to explicitly define previously implicit header include (Kittywhiskers Van Gogh) c768bfa tests: Calculate fees more similarly to CFeeRate::GetFee (Andrew Chow) f66bc42 tests: Test for assertion when feerate is rounded down (Andrew Chow) bd7e08e fees: Always round up fee calculated from a feerate (Andrew Chow) 227ae65 wallet: fix segfault by avoiding invalid default-ctored `external_spk_managers` entry (Sebastian Falbesoner) 282863a refactor: include a missing <limits> header in fs.cpp (Joan Karadimov) 7febe4f consensus: don't call GetBlockPos in ReadBlockFromDisk without lock (Jon Atack) c671c6f the result of CWallet::IsHDEnabled() was initialized with true. (Saibato) a5a1538 build, qt: Fix typo in QtInputSupport check (Hennadii Stepanov) c95b188 system: skip trying to set the locale on NetBSD (fanquake) c1cdedd guix: Fix powerpc64(le) dynamic linker name (Carl Dong) 92d44ff doc: Add 23061 release notes (MarcoFalke) db76db7 Fix (inverse) meaning of -persistmempool (MarcoFalke) 85c78e0 build: Restrict check for CRC32C intrinsic to aarch64 (W. J. van der Laan) Pull request description: Collecting backports for the 22.1 release. Currently: * #23045 * #23061 * #23148 * #22390 * #22820 * #22781 * #22895 * #23335 * #23333 * #22949 * #23580 * #23504 * #24239 ACKs for top commit: achow101: ACK 269553f Tree-SHA512: b3a57ea241be7a83488eeb032276f4cf82a0987aada906a82f94a20c4acf9f2397708249dcecbe1c7575e70d09c60b835233d4718af4013c7bc58896c618274c

…s.cpp 077a875 refactor: include a missing <limits> header in fs.cpp (Joan Karadimov) Pull request description: I get this compilation error on versions `0.21.1` and `22.0`: ``` fs.cpp: In member function 'bool fsbridge::FileLock::TryLock()': fs.cpp:123:89: error: 'numeric_limits' is not a member of 'std' 123 | if (!LockFileEx(hFile, LOCKFILE_EXCLUSIVE_LOCK | LOCKFILE_FAIL_IMMEDIATELY, 0, std::numeric_limits<DWORD>::max(), std::numeric_limits<DWORD>::max(), &overlapped)) { | ^~~~~~~~~~~~~~ fs.cpp:123:109: error: expected primary-expression before '>' token 123 | if (!LockFileEx(hFile, LOCKFILE_EXCLUSIVE_LOCK | LOCKFILE_FAIL_IMMEDIATELY, 0, std::numeric_limits<DWORD>::max(), std::numeric_limits<DWORD>::max(), &overlapped)) { | ^ fs.cpp:123:112: error: '::max' has not been declared; did you mean 'std::max'? 123 | if (!LockFileEx(hFile, LOCKFILE_EXCLUSIVE_LOCK | LOCKFILE_FAIL_IMMEDIATELY, 0, std::numeric_limits<DWORD>::max(), std::numeric_limits<DWORD>::max(), &overlapped)) { | ^~~ | std::max In file included from C:/dev/msys64/mingw64/include/c++/11.2.0/bits/char_traits.h:39, from C:/dev/msys64/mingw64/include/c++/11.2.0/string:40, from ./fs.h:9, from fs.cpp:5: C:/dev/msys64/mingw64/include/c++/11.2.0/bits/stl_algobase.h:300:5: note: 'std::max' declared here 300 | max(const _Tp& __a, const _Tp& __b, _Compare __comp) | ^~~ fs.cpp:123:124: error: 'numeric_limits' is not a member of 'std' 123 | if (!LockFileEx(hFile, LOCKFILE_EXCLUSIVE_LOCK | LOCKFILE_FAIL_IMMEDIATELY, 0, std::numeric_limits<DWORD>::max(), std::numeric_limits<DWORD>::max(), &overlapped)) { | ^~~~~~~~~~~~~~ fs.cpp:123:144: error: expected primary-expression before '>' token 123 | if (!LockFileEx(hFile, LOCKFILE_EXCLUSIVE_LOCK | LOCKFILE_FAIL_IMMEDIATELY, 0, std::numeric_limits<DWORD>::max(), std::numeric_limits<DWORD>::max(), &overlapped)) { | ^ fs.cpp:123:147: error: '::max' has not been declared; did you mean 'std::max'? 123 | if (!LockFileEx(hFile, LOCKFILE_EXCLUSIVE_LOCK | LOCKFILE_FAIL_IMMEDIATELY, 0, std::numeric_limits<DWORD>::max(), std::numeric_limits<DWORD>::max(), &overlapped)) { | ^~~ | std::max In file included from C:/dev/msys64/mingw64/include/c++/11.2.0/bits/char_traits.h:39, from C:/dev/msys64/mingw64/include/c++/11.2.0/string:40, from ./fs.h:9, from fs.cpp:5: C:/dev/msys64/mingw64/include/c++/11.2.0/bits/stl_algobase.h:300:5: note: 'std::max' declared here 300 | max(const _Tp& __a, const _Tp& __b, _Compare __comp) | ^~~ ``` 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. ACKs for top commit: fanquake: ACK 077a875 - Thanks. hebasto: ACK 077a875 Tree-SHA512: 2289cb72fa3a28470f4250833be66079482d1392189b1e4679330dad109a8ae67b1d6d51cb635dbc1947c00c6c4cfbc4d7c9ae02269b932cfa1475583adaf73d

... needed for std::numeric_limits<T>::max on WIN32 Github-Pull: bitcoin#23335 Rebased-From: 077a875

fanquake · 2022-06-09T11:57:29Z

Backported to 0.21 in #25318.

efb9f00 build: Restrict check for CRC32C intrinsic to aarch64 (W. J. van der Laan) cfb08c3 refactor: include a missing <limits> header in fs.cpp (Joan Karadimov) Pull request description: There might not be another 0.21.x release, however these are both straight forward changes. If this isn't merged, then the pulls can remain untagged for needing backport. Backports: - #23045 - #23335 ACKs for top commit: laanwj: ACK efb9f00 LarryRuane: utACK efb9f00 Tree-SHA512: 09be8f8ce90f862e2d408c5707a8387ca828fdd05a9814cfed5236030a3b33012e7d7a557c2ee3989db26922ad45cb8a307bdeba7ac8e34b5f21f0d46eda1955

refactor: include a missing <limits> header in fs.cpp

077a875

... needed for std::numeric_limits<T>::max on WIN32

fanquake changed the title ~~Include a missing header~~ refactor: include a missing <limits> header in fs.cpp Oct 22, 2021

fanquake approved these changes Oct 22, 2021

View reviewed changes

DrahtBot added the Refactoring label Oct 22, 2021

maflcko added Needs backport (0.21) labels Oct 22, 2021

hebasto approved these changes Oct 22, 2021

View reviewed changes

maflcko merged commit 4833d1f into bitcoin:master Oct 22, 2021

fanquake pushed a commit to fanquake/bitcoin that referenced this pull request Oct 22, 2021

refactor: include a missing <limits> header in fs.cpp

be1a57d

... needed for std::numeric_limits<T>::max on WIN32 Github-Pull: bitcoin#23335 Rebased-From: 077a875

fanquake removed the Needs backport (22.x) label Oct 22, 2021

joankaradimov deleted the include-missing-header branch October 22, 2021 21:27

fanquake mentioned this pull request Nov 22, 2021

[22.x] Backports for 22.x #23276

Merged

str4d mentioned this pull request Dec 6, 2021

Backport changes to src/fs.{cpp,h} zcash/zcash#5406

Open

fanquake pushed a commit to fanquake/bitcoin that referenced this pull request Feb 15, 2022

refactor: include a missing <limits> header in fs.cpp

282863a

... needed for std::numeric_limits<T>::max on WIN32 Github-Pull: bitcoin#23335 Rebased-From: 077a875

reddink mentioned this pull request May 26, 2022

[4.22.x] Backports for 4.22.x reddcoin-project/reddcoin#20

Merged

fanquake pushed a commit to fanquake/bitcoin that referenced this pull request Jun 9, 2022

refactor: include a missing <limits> header in fs.cpp

cfb08c3

... needed for std::numeric_limits<T>::max on WIN32 Github-Pull: bitcoin#23335 Rebased-From: 077a875

fanquake mentioned this pull request Jun 9, 2022

0.21 Backports #25318

Merged

fanquake removed the Needs backport (0.21) label Jun 9, 2022

delta1 added a commit to delta1/elements that referenced this pull request May 14, 2023

Merge 4833d1f into merged_master (Bitcoin PR bitcoin/bitcoin#23335)

dcf9336

bitcoin locked and limited conversation to collaborators Jun 9, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

refactor: include a missing <limits> header in fs.cpp#23335
maflcko merged 1 commit intobitcoin:masterfrom
joankaradimov:include-missing-header

joankaradimov commented Oct 21, 2021 •

edited

Loading

Uh oh!

theuni commented Oct 21, 2021

Uh oh!

joankaradimov commented Oct 21, 2021

Uh oh!

theuni commented Oct 21, 2021

Uh oh!

joankaradimov commented Oct 22, 2021

Uh oh!

fanquake commented Oct 22, 2021

Uh oh!

joankaradimov commented Oct 22, 2021

Uh oh!

fanquake left a comment

Uh oh!

hebasto left a comment

Uh oh!

fanquake commented Oct 22, 2021

Uh oh!

fanquake commented Jun 9, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Conversation

joankaradimov commented Oct 21, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

theuni commented Oct 21, 2021

Uh oh!

joankaradimov commented Oct 21, 2021

Uh oh!

theuni commented Oct 21, 2021

Uh oh!

joankaradimov commented Oct 22, 2021

Uh oh!

fanquake commented Oct 22, 2021

Uh oh!

joankaradimov commented Oct 22, 2021

Uh oh!

fanquake left a comment

Choose a reason for hiding this comment

Uh oh!

hebasto left a comment

Choose a reason for hiding this comment

Uh oh!

fanquake commented Oct 22, 2021

Uh oh!

fanquake commented Jun 9, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

joankaradimov commented Oct 21, 2021 •

edited

Loading