19:00:16 <laanwj> #startmeeting 19:00:16 <core-meetingbot`> Meeting started Thu Mar 10 19:00:15 2022 UTC. The chair is laanwj. Information about MeetBot at https://bitcoin.jonasschnelli.ch/ircmeetings. 19:00:16 <core-meetingbot`> Available commands: action commands idea info link nick 19:00:20 <jonatack> hi 19:00:25 <sdaftuar> hi 19:00:33 <michaelfolkson> hi 19:00:34 <laanwj> #bitcoin -core-dev Meeting: achow101 _aj_ amiti ariard BlueMatt cfields Chris_Stewart_5 darosior digi_james dongcarl elichai2 emilengler fanquake fjahr gleb glozow gmaxwell gwillen hebasto instagibbs jamesob jarolrod jb55 jeremyrubin jl2012 jnewbery jonasschnelli jonatack jtimon kallewoof kanzure kvaciral laanwj larryruane lightlike luke-jr maaku marcofalke meshcollider michagogo moneyball 19:00:36 <laanwj> morcos nehan NicolasDorier paveljanik petertodd phantomcircuit promag provoostenator ryanofsky sdaftuar sipa vasild 19:00:38 <dongcarl> hi 19:00:42 <sipa> hi 19:00:46 <kvaciral[m]> hi 19:00:46 <provoostenator> hi 19:00:56 <lightlike> hi 19:01:02 <hebasto> hi 19:01:03 <achow101> hi 19:01:30 <laanwj> PSA: 23.0rc1 had some build issues, so 23.0rc2 was tagged today with fixes for that, please start your guix builds if you haven't yet 19:01:32 <jeremyrubin> hello 19:01:35 <darosior> hi 19:02:01 <laanwj> any topics for today? none have been proposed during the week with #proposedmeetingtopic, as far as i can see 19:02:51 <MarcoFalke> Can I haz #24080 for hi-prio pls? 19:02:52 <gribble> https://github.com/bitcoin/bitcoin/issues/24080 | policy: Remove unused locktime flags by MarcoFalke ÷ Pull Request #24080 ÷ bitcoin/bitcoin ÷ GitHub 19:03:00 <laanwj> we're not at the high prio topic yet, but sure 19:03:21 <MarcoFalke> ups 19:03:40 <glozow> hi 19:04:05 <laanwj> #topic High priority for review 19:04:05 <core-meetingbot`> topic: High priority for review 19:04:11 <laanwj> MarcoFalke: added 19:04:14 <MarcoFalke> thx 19:04:15 <sipsorcery> hi 19:04:32 <provoostenator> I'd still like to get this into v23 if if survives more review: https://github.com/bitcoin-core/gui/pull/555 19:04:45 <provoostenator> (already tagged with the milestone, so I guess not high prio list needed) 19:04:46 <laanwj> is there anything else people want to add to high priority for review, or to remove, or that is ready for merge? 19:05:01 <dongcarl> Could I have #24322 for high prio? Will update within the next hour with the changes just discussed in #bitcoin-core-builds (shouldn't change much) 19:05:03 <gribble> https://github.com/bitcoin/bitcoin/issues/24322 | [kernel 1/n] Introduce initial `libbitcoinkernel` by dongcarl ÷ Pull Request #24322 ÷ bitcoin/bitcoin ÷ GitHub 19:05:17 <laanwj> provoostenator: is that a bugfix? 19:06:16 <laanwj> dongcarl: added 19:06:24 <dongcarl> thx! 19:06:46 <provoostenator> laanwj: yes 19:07:03 <provoostenator> Well at least it fixes a regression compared to v22 19:07:37 <laanwj> provoostenator: makes sense then, it wasn't clear to me from the title enabling the send button sounded more like a feature 19:08:13 <provoostenator> Good point, renamed to more dramatic word choice: restore 19:08:31 <laanwj> yess 19:09:13 <fjahr> hi 19:09:34 <MarcoFalke> (off-topic: My C++20 patch is now ready #24169. Not sure if this needs discussion, if it does, then this is my topic suggestion) 19:09:35 <gribble> https://github.com/bitcoin/bitcoin/issues/24169 | build: Add --enable-c++20 option by MarcoFalke ÷ Pull Request #24169 ÷ bitcoin/bitcoin ÷ GitHub 19:11:30 <laanwj> good to know, i don't think that particular PR needs discussion, only if the goal is to make c++20 a minimum requirement 19:11:46 <laanwj> but i suppose that's still some time away 19:12:27 <sipa> @MarcoFalke cool 19:12:41 <MarcoFalke> Yes, the minimum will be changed later (not sure when) 19:12:45 <sipa> (i don't think there is much more to discuss about that either) 19:13:18 <laanwj> is there any pressing reason for c++20? 19:13:27 <laanwj> in the context of our project 19:13:31 <MarcoFalke> Yeah, the pull shouldn't affect anyone unless they opt-in 19:14:05 <MarcoFalke> laanwj: named arguments (which work since clang 3) were only officially added in C++20 19:14:25 <laanwj> ok 19:14:31 <jonatack> will try it. there are also some niceties in std::chrono and std::algorithm that I would have liked to use 19:15:00 <jonatack> (so far) 19:15:08 <sipa> it's not like you can actually use them with this already, except inside #if #else variants. 19:15:30 <sipa> This is really just to make sure that our codebase is c++20 compliant (as well as c++17 compliant) 19:15:35 <MarcoFalke> See also #23363 19:15:36 <gribble> https://github.com/bitcoin/bitcoin/issues/23363 | Discussion: Upgrading to C++20 ÷ Issue #23363 ÷ bitcoin/bitcoin ÷ GitHub 19:16:28 <laanwj> it seems a bit eary to break compatibility with compilers again 19:16:28 <dongcarl> MarcoFalke: Named arguments would really help my branch removing argsmanager from validation, where I introduce a few Options structs, alas perhaps too early to bump min C++ 19:17:25 <laanwj> sipa: yes, making sure it can be compiled with c++20 makes sense of course 19:17:49 <MarcoFalke> Named arguments choke only the msvc C++17 compiler, so as a hack we could bump msvc to C++20 and then start using named args with C++17 on clang/gcc 19:17:51 <_aj_> dongcarl: have you seen https://github.com/bitcoin/bitcoin/pull/24032/commits/3d3724e95b34931a4de8d31450f33ef0a32965e7#diff-ff53e63501a5e89fd650b378c9708274df8ad5d38fcffa6c64be417c4d438b6dR376-R382 ? gives fake named arguments 19:17:51 <MarcoFalke> hides 19:18:24 <sipa> GCC only fully supports C++20 as of GCC 11, which was released 9 momths ago. It'll be a few years I think before we can assume common platforms have it available. 19:18:51 <laanwj> right 19:19:03 <jonatack> oh 19:19:05 <sipa> or even some parts in GCC 12 19:19:07 <dongcarl> sipa: True 19:19:22 <dongcarl> MarcoFalke: No flag on msvc to enable _just_ named arguments? 19:19:23 <MarcoFalke> At least Ubuntu/Debian seem agressive in backporting compilers 19:19:40 <laanwj> i dont't think we should be jumping on bumping the minimum gcc faster and faster 19:19:47 <dongcarl> _aj_: Lol very cool hack! 19:20:21 <sipa> It's true that the guix based build allows us to go a bit faster, but still..m 19:21:09 <jeremyrubin> sends aj to c++ jail 19:21:54 <sipa> and removes all his friend classes? 19:22:01 <MarcoFalke> Right, I am not too happy that this needs bumping so far 19:22:33 <MarcoFalke> Let's see how many people will complain about the gcc-8 minimum in 23.x 19:22:43 <laanwj> _aj_: scary lol 19:24:26 <MarcoFalke> Though, designated initializers is already implemented in gcc-8 19:26:46 <laanwj> gcc 8 was released in 2018, hopefully not too many 19:27:39 <MarcoFalke> dongcarl: Not aware of a flag for that, but I don't think our msvc users are running that old software, so at least for them C++20 shouldn't be an issue 19:28:12 <laanwj> and yes guix allows bumping faster, though, it's generally nice if it's possible to build bitcoind with a platforms' existing compiler as-is 19:29:22 <laanwj> for MSVC there's less reason to support old versions 19:30:15 <dongcarl> +1 "it's generally nice if it's possible to build bitcoind with a platforms' existing compiler as-is" 19:30:30 <sipa> indeed 19:31:12 <_aj_> jeremyrubin: i'm not stuck in here with c++, c++ is stuck in here with me 19:31:39 <laanwj> any other topics for today? 19:32:29 <MarcoFalke> So would people object using C++17 + designated initializers? 19:33:11 <sipa> I'm not very much a fan of enabling features too selectively, but I guess if there is a good reason? 19:33:15 <jonatack> I think I agree with sipa's comment against whitelisting features granularly 19:33:17 <dongcarl> I'd support it since it's been in gcc and clang for so long, but I don't know the msvc situation 19:33:26 <sipa> *granularly 19:33:43 <MarcoFalke> Seems slightly preferable to me than to implement designated initializers ourselves 19:33:44 <laanwj> it's a bit weird to conform to a standard but then make an exception, then again, like with browsers it seems that there's so little competition that "if it works with gcc and clang" is all that matters... 19:34:26 <jeremyrubin> building our own c++ compiler seems like something we'll do at some point 19:34:52 <laanwj> so i don't have a big problem with it either 19:34:56 <dongcarl> I found it very useful in mostly heterogenous structs to make sure I'm not setting the wrong element, or to set only a subset of elements and leave the rest to default. 19:35:00 <laanwj> jeremyrubin: who is this we 19:35:10 <MarcoFalke> Just saying, if we want #24032, then it seems we want designated initializers. If not, then well no need to discuss ;) 19:35:12 <gribble> https://github.com/bitcoin/bitcoin/issues/24032 | Refactor vDeployments setup to avoid uninitialized variables by ajtowns ÷ Pull Request #24032 ÷ bitcoin/bitcoin ÷ GitHub 19:35:19 <dongcarl> You could do it in other ways, but it's much nicer with designated initializers 19:36:14 <dongcarl> other ways = clang-tidy annotations / Options foo{}; foo.this_particular_item = 0; foo.that_particular_item = 5; 19:36:22 <MarcoFalke> yeah, a workaround could be to add a constructor to the struct that takes all arguments and then use clang-tidy's named argument check 19:37:06 <_aj_> (foo.this=1; foo.that=2; prevents "foo" from being const) 19:37:19 <dongcarl> _aj_: TRUE! I ran into that ytd 19:37:36 <jeremyrubin> _aj_: you can rebind it after to a const? 19:37:40 <jeremyrubin> if it's moveable 19:37:50 <_aj_> jeremyrubin: or wrap it in a function, sure 19:38:00 <laanwj> so to be clear, using designated initializers works right now, without any build system changes? 19:38:15 <laanwj> and without bumping minimum compiler versions 19:38:34 <MarcoFalke> const Options foo{[]{ Options _foo{}; _foo.a = 0; _foo.b = 1; return _foo; }()}; 19:39:02 <MarcoFalke> laanwj: Yes, we already did that a long time ago 19:39:08 <dongcarl> MarcoFalke: 𤮠19:39:29 <MarcoFalke> laanwj: Only found out when we started compiling the code (fuzz tests) with msvc 19:39:33 <dongcarl> MarcoFalke: Oh it doesn't fail on msvc? It's just review that's stopping designated initializers? 19:39:37 <laanwj> i mean in that case i don't see a big problem with using them 19:40:19 <MarcoFalke> dongcarl: It does fail msvc. Though, bumping msvc to c++20 should fix that issue. 19:40:23 <laanwj> bump the msvc requirement to c++20 19:40:25 <_aj_> https://en.cppreference.com/w/cpp/compiler_support -- lists Apple's Clang as only having partial support 19:40:25 <laanwj> right 19:41:21 <MarcoFalke> Jup, but that should be the same partial support that clang-7 has 19:41:35 <laanwj> so we can only use the partial support 19:41:54 <MarcoFalke> Right 19:41:57 <laanwj> if that isn't enough then there's no need to bother 19:42:04 <dongcarl> wonders if we should start using {fmt} in preparation for C++20 std::format 19:42:27 <_aj_> sgtm 19:43:12 <MarcoFalke> dongcarl: std::format lands with improvements in C++23, so is it ready for production in C++20 already? 19:43:45 <Earnest> Is there an issue/pr for dumpwallet with descriptor wallets? 19:44:07 <dongcarl> MarcoFalke: oh perhaps not, haven't looked into it yet. {fmt} is compatible with C++20 std::format and has users, so I assume it's somewhat usable? 19:44:26 <Earnest> Or will that be replaced with new 'listdescriptors true' rpc? 19:44:39 <laanwj> i dont think there is an urgent reason to switch to another way of string formatting 19:44:55 <MarcoFalke> The designated initializers partial support doesn't allow braces, which no one should be using anyway unless required 19:45:24 <dongcarl> laanwj: Right, not urgent at all 19:47:27 <laanwj> replacing tinyformat is something we can do after c++20 or even c++23 19:48:01 <dongcarl> Yup! 19:48:23 <bitcoin-git> [bitcoin] mzumsande opened pull request #24527: test: set segwit height back to 0 on regtest (master...202203_regtest_segwitheight) https://github.com/bitcoin/bitcoin/pull/24527 19:48:29 <_aj_> replacing tinyformat sounds like a lot of lines of code to change 19:48:54 <laanwj> but it could be mechanical, maybe 19:49:14 <laanwj> given that format strings map in some way 19:49:45 <laanwj> in any case, it's not urgent 19:49:47 <laanwj> any other topics? 19:51:41 <laanwj> #endmeeting