From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp.polymtl.ca (smtp.polymtl.ca [132.207.4.11]) by sourceware.org (Postfix) with ESMTPS id 0DCD33858D28 for ; Wed, 16 Mar 2022 02:06:14 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 0DCD33858D28 Received: from simark.ca (simark.ca [158.69.221.121]) (authenticated bits=0) by smtp.polymtl.ca (8.14.7/8.14.7) with ESMTP id 22G26669023891 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Tue, 15 Mar 2022 22:06:11 -0400 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp.polymtl.ca 22G26669023891 Received: from [10.0.0.11] (192-222-157-6.qc.cable.ebox.net [192.222.157.6]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 29FC51F0D2; Tue, 15 Mar 2022 22:06:06 -0400 (EDT) Message-ID: <81442d2a-055a-961a-c804-ed743bc04c72@polymtl.ca> Date: Tue, 15 Mar 2022 22:06:05 -0400 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.6.1 Subject: Re: [PATCH 4/4] gdb: make internalvar use a variant Content-Language: en-US To: Pedro Alves , Tom Tromey , Simon Marchi via Gdb-patches References: <20220201140717.3046952-1-simon.marchi@polymtl.ca> <20220201140717.3046952-5-simon.marchi@polymtl.ca> <87bkylu1ow.fsf@tromey.com> <205f0a37-e832-2e25-32b6-266acc33753d@palves.net> From: Simon Marchi In-Reply-To: <205f0a37-e832-2e25-32b6-266acc33753d@palves.net> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Wed, 16 Mar 2022 02:06:06 +0000 X-Spam-Status: No, score=-3032.9 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, NICE_REPLY_A, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_PASS, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 16 Mar 2022 02:06:15 -0000 > It requires more C++ expertise than most of other things to know to write such code. Something that should be > simple is complicated. C++ is really lacking proper pattern matching and builtin variant types, to make > variants easy to use. But if we use it in a contained way like basically glorified tagged union to get rid > of some boilerplace we have to write today, I guess it's OK. Yes, that was my thought. The intent is replacing those instances of "enum kind + union data" with something that can hold non-trivial objects, calls constructors / destructors on data fields automatically, validates that we don't access a wrong (inactive) field. That being said, it's likely that many instances of this pattern are in structures that aren't even new'ed / delete'd today, so it's not as if we could use variants in them tomorrow. > How debuggeable is nonstd::variant? When printing a tagged union in GDB, it's easy to figure out the > union's current value. What does printing a nonstd::variant (not mapped to the std variant) look > like? With nonstd: $1 = { next = 0x60600000ff20, name = 0x602000085330 "salut", v = { data = { __data = '\276' , __align = {} }, type_index = 0 '\000' } } With std: $1 = { next = 0x60600000ff20, name = 0x602000085350 "salut", v = std::variant [index 0] = {{}} } > AFAICT from variant.cpp source file, the variant storage is an untyped buffer: > > enum { data_align = detail::typelist_max_alignof< variant_types >::value }; > > using aligned_storage_t = typename std::aligned_storage< data_size, data_align >::type; > aligned_storage_t data; > > so I guess printing a nonstd::variant results in pretty opaque output. We'd need a pretty > printer to fix this. Or maybe we just assume that people developing/debugging GDB build > it against a C++17 or higher compiler? (Not sure that's a great assumption.) Do you know off-hand if the std::variant pretty printer is supposed to show the active data field? > I wonder whether we should do: > > namespace gdb { > using namespace nonstd; > } > > and then use gdb::variant throughout, instead of "nonstd::variant". At least "gdb" > is three letters, and thus less code/indentation churn when we someday switch to "std". > Also less churn if we move to some other variant type, though not sure that's likely > after we start using one. Good idea. But now I'm not even convinced myself of the usefulness of having an std::variant-like in the tree. The internalvar change is really just an example, but it doesn't really need changing (and Tom suggeted what's probably a better way to change it if we wanted to). I wanted to use a variant in some TUI patch that I started, but haven't finished. So, I guess we have it here as a reference if the need ever comes up again. Simon