randomenv: Fix MinGW dllimport warning for environ #45
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
As reported by hebasto in bitcoin#33545, newer libc++ versions implementing https://wg21.link/lwg3430 will no longer implicitly convert `fs::path` objects to `std::filesystem::path` objects when constructing `std::ifstream` and `std::ofstream` types. This is not a problem in Unix systems since `fs::path` objects use `std::string` as their native string type, but it causes compile errors on Windows which use `std::wstring` as their string type, since `fstream`s can't be constructed from `wstring`s. Fix the windows libc++ compile errors by adding a new `fs::path::std_path()` method and using it construct `fstream`s more portably. Additionally, delete `fs::path`'s implicit `native_string` conversion so these errors will not go undetected in the future, even though there is not currently a CI job testing Windows libc++ builds.
These overloads were needed to allow passing `fs::path` objects directly to libstdc++'s `fstream` constructors, but after the previous commit, there is no longer any remaining code that does pass `fs::path` objects to `fstream` constructors. Writing new code which does this is also discouraged because the standard has been updated in https://wg21.link/lwg3430 to disallow it. Dropping these also means its no longer possible to pass `fs::path` arguments directly to `fstream::open` in libstdc++, which is somewhat unfortunate but not a big loss because it is already not possible to pass them to the constructor. So this commit updates `fstream::open` calls. Additionally, this change required updates to src/bitcoin.cpp since it was relying on the overloaded filename() method.
Extends 7703884 to guard environ declaration on all Windows builds, not just MSVC. MinGW also defines environ as a macro that expands to a dllimport function, causing the same inconsistent linkage warning. Use WIN32 instead of _MSC_VER to match the platform-specific guards already used throughout the file.
This was referenced Oct 8, 2025
Draft
hebasto
added a commit
to bitcoin/bitcoin
that referenced
this pull request
Oct 22, 2025
9610b0d randomenv: Fix MinGW dllimport warning for `environ` (Lőrinc) Pull request description: Related to #33550 (comment) Extends 7703884 to guard environ declaration on all Windows builds, not just MSVC. In the `mingw-w64` headers (used by `llvm-mingw`), `environ` is defined as a macro which expands through [`_environ`](https://github.com/msys2-contrib/mingw-w64/blob/cdb052f1d4056cd510cb83197b55868427b87476/mingw-w64-headers/crt/stdlib.h#L262-L264) to `(* __p__environ())`, a call to a `dllimport` function, causing the same inconsistent linkage warning as MSVC. Use `WIN32` instead of `_MSC_VER` to match the platform-specific guards already used throughout the file. The warning occurs with `llvm-mingw` (both `UCRT` and `MSVCRT` variants as tested by Hebasto), but not with the `mingw-w64` toolchain currently used in CI (as mentioned by fanquake). ---- The error was reproduced by adding a temporary [nightly build](l0rinc/bitcoin-core-nightly#4) pointing to l0rinc#45. On `master` the failure can be seen in l0rinc/bitcoin-core-nightly#2 before: https://github.com/l0rinc/bitcoin-core-nightly/actions/runs/18327936488/job/52196728885?pr=2 <details> <summary>Details</summary> ``` /home/runner/work/bitcoin-core-nightly/bitcoin-core-nightly/src/randomenv.cpp:61:15: warning: '__p__environ' redeclared without 'dllimport' attribute: previous 'dllimport' ignored [-Winconsistent-dllimport] 61 | extern char** environ; // NOLINT(readability-redundant-declaration): Necessary on some platforms | ^ /home/runner/work/bitcoin-core-nightly/bitcoin-core-nightly/llvm_mingw_toolchain/aarch64-w64-mingw32/include/stdlib.h:656:17: note: expanded from macro 'environ' 656 | #define environ _environ | ^ /home/runner/work/bitcoin-core-nightly/bitcoin-core-nightly/llvm_mingw_toolchain/aarch64-w64-mingw32/include/stdlib.h:225:21: note: expanded from macro '_environ' 225 | #define _environ (* __p__environ()) | ^ /home/runner/work/bitcoin-core-nightly/bitcoin-core-nightly/llvm_mingw_toolchain/aarch64-w64-mingw32/include/stdlib.h:221:27: note: previous declaration is here 221 | _CRTIMP char ***__cdecl __p__environ(void); | ^ /home/runner/work/bitcoin-core-nightly/bitcoin-core-nightly/llvm_mingw_toolchain/aarch64-w64-mingw32/include/stdlib.h:221:3: note: previous attribute is here 221 | _CRTIMP char ***__cdecl __p__environ(void); | ^ /home/runner/work/bitcoin-core-nightly/bitcoin-core-nightly/llvm_mingw_toolchain/aarch64-w64-mingw32/include/_mingw.h:52:40: note: expanded from macro '_CRTIMP' 52 | # define _CRTIMP __attribute__ ((__dllimport__)) | ^ 1 warning generated. ``` </details> after: https://github.com/l0rinc/bitcoin-core-nightly/actions/runs/18329616268/job/52201940831?pr=4 <details> <summary>Details</summary> ``` [ 28%] Building CXX object src/util/CMakeFiles/bitcoin_util.dir/__/randomenv.cpp.obj ``` </details> Note that there are some other remaining warnings in the logs that will be fixed in separate PRs ACKs for top commit: sipa: utACK 9610b0d if this makes the compilers happy laanwj: Code review ACK 9610b0d hebasto: re-ACK 9610b0d. Tree-SHA512: a9e39d288b663ed24cbbbae228850e6f02d417d8781a3ac3d0b3db0b7ff734bbd62fddb9f57b8f77daab4e9c016ff66906ebc5fb2de7635ef539ef7f4dc2eaba
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Extends 7703884 to guard environ declaration on all Windows builds, not just
MSVC.MinGW also defines environ as a macro that expands to a dllimport function, causing the same inconsistent linkage warning.
Use
WIN32instead of_MSC_VERto match the platform-specific guards already used throughout the file.