fix: make conversion to address explicit#487
Conversation
|
Thanks. I would guess that was caused by the movement of these config classes to system. But I'm curious why it hasn't been encountered in any of the system, network, or node CI builds. |
|
|
||
| address::address(const asio::endpoint& uri) NOEXCEPT | ||
| : address(endpoint{ uri }) | ||
| : address(address{ endpoint{ uri } }) |
There was a problem hiding this comment.
I'm having a hard time figuring out why this should make a difference, as there are just two constructions of the same address object, where the outer is a move construction and the inner is a network::config::address construction from a network::config::endpoint object (which is what it was before the change). Am I missing something?
|
In the console output the error messages appear to be truncated. This is the attempt (default move) that should be matching, as none are Because
|
|
Ahh, maybe it's the first This is unnecessarily restrictive and not consistent with our style. The const object cannot be accepted as a move argument (though it should be accepted by the default copy constructor). Normally there would only be a const return for reference cast operator. Since I can't repro could you please try this with this change (and the corresponding change in endpoint.cpp)? In either case it looks like a compiler issue (and is passing in all CI) but the const should be removed and might resolve it. Note that the compiler does attempt to match the copy constructor: which should succeed. But by wrapping the address with a second construction the |
Remove const from address, authority return types. This fixes a compiler error when building with clang.
Thanks for digging in. I confirmed this also fixes the compiler error I'm seeing, and it makes sense why the original patch was working, i.e., stripping the const.
I'm not familiar with your CI, but I was able to confirm on my linux machine that this is specific to clang by building with both clang and gcc. The error only happens with clang , and is then resolved with this patch applied. If interested, this is what I'm using to build / reproduce the error: https://github.com/josibake/nix-libbitcoin |
|
You can see our CI builds on the GitHub Actions tab for each repo. Matrix includes Ubuntu, Windows, macOS, gcc, clang, msvc, Xcode, make, cmake, msbuild. Can you post the clang version that failed? |
This is using the ClangStdenv toolchain from nix. If it's not the version itself, its possible that in their wrapper they are setting some flags that are causing the error. Haven't had a chance to dig further but will revisit at some point if I get a chance. |
|
We test at clang16. |
2 participants
When building on macOS via nix, I get the following error:
.. which seems to be caused by clang being more strict than gcc and not allowing for the implicit conversion of an endpoint to an address. Adding this small patch fixed the issue for me, so I figured I'd upstream it here.