From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id 9ZlEFuNtiWCAJwAAWB0awg (envelope-from ) for ; Wed, 28 Apr 2021 10:14:59 -0400 Received: by simark.ca (Postfix, from userid 112) id 4CAA31F11C; Wed, 28 Apr 2021 10:14:59 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-0.7 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,MAILING_LIST_MULTI,RDNS_DYNAMIC,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.2 Received: from sourceware.org (ip-8-43-85-97.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id A529A1E01F for ; Wed, 28 Apr 2021 10:14:58 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 2BD2D393BC01; Wed, 28 Apr 2021 14:14:58 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 2BD2D393BC01 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1619619298; bh=Joo9Ca7LCjLyX00nkGyL16gEf/zxLL1H/ZNreHbdjos=; h=Subject:To:References:Date:In-Reply-To:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=abY3bTpMnOa0CVE7rJim/5Bchtj7cmGBX8bDOWzJDnbBdNe2oFs2aryacRzCsTkVT 3r+DawO+Hulr1oP8bAw5P7PzCPIM3iyko4VtW15vz5GkUl3I60J+N+eGZbwJAdKz6d wM0RphxFjKCyiLewGsEqIUKkxorJYepjLYQ1gsbE= Received: from smtp.polymtl.ca (smtp.polymtl.ca [132.207.4.11]) by sourceware.org (Postfix) with ESMTPS id BA8D7385701E for ; Wed, 28 Apr 2021 14:14:55 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org BA8D7385701E 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 13SEEhHw009266 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 28 Apr 2021 10:14:48 -0400 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp.polymtl.ca 13SEEhHw009266 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 6582E1E01F; Wed, 28 Apr 2021 10:14:43 -0400 (EDT) Subject: Re: [PATCH 03/43] Move frame context info to dwarf_expr_context To: Zoran Zaric , gdb-patches@sourceware.org References: <20210301144620.103016-1-Zoran.Zaric@amd.com> <20210301144620.103016-4-Zoran.Zaric@amd.com> <7f00dde3-c7dc-f839-bf8d-035f18d65ce1@polymtl.ca> <0edee4c1-9a04-9bf5-7d44-0973c675462b@amd.com> Message-ID: <5a2c4d75-2059-cc93-6581-59cde0d6ef40@polymtl.ca> Date: Wed, 28 Apr 2021 10:14:43 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.7.1 MIME-Version: 1.0 In-Reply-To: <0edee4c1-9a04-9bf5-7d44-0973c675462b@amd.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Wed, 28 Apr 2021 14:14:43 +0000 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: , From: Simon Marchi via Gdb-patches Reply-To: Simon Marchi Errors-To: gdb-patches-bounces@sourceware.org Sender: "Gdb-patches" >> On an unrelated topic, things like ensure_have_frame always worry me >> because I'm scared I'll forget to call it where necessary. An >> alternative could be to make sure we get the frame through a getter that >> throws if frame == nullptr. >> >> Basically, a tiny class (could be internal to dwarf_expr_context): >> >> struct frame_safe >> { >> frame_safe (frame_info *frame) >> : m_frame (frame) >> {} >> >> frame_info *frame (const char *op) >> { >> if (m_frame == nullptr) >> ... throw ... >> >> return m_frame; >> } >> >> private: >> frame_info *m_frame; >> }; >> >> dwarf_expr_context would have a `frame_safe m_frame` field and a >> `frame (const char *op)` getter that just does >> `return m_frame->frame (op)`. >> >> This way, it's impossible to get the frame and forgetting an >> ensure_have_frame call. > > True, but then you end up with even more "switch" statements in different parts of the gdb that one needs to update every time they add a new operation and this is one of the problems with the current design as is. I don't understand what you mean, why it would mean adding more switch statements. In practice, it would just mean changing `this->frame` to `this->frame ()`. > This would force us to add two new places (one for frame and one for compilation unit) and there will be more in the future when we add a concept of a thread to the context. > > At least with the current implementation the implementer of the case statement that process that operation needs to know which part of the context has to be present for that operation, which is clearly defined in the DWARF standard as part of that operations definition (or at least it will be after our extensions). > > Another problem with the new class approach is that it makes the information about what each operation needs convoluted and hidden which in my mind adds even bigger chance of someone forgetting to add their operation to the query calls. What do you mean by "add their operation to the query calls"? I don't think see how it helps to make it explicit which operation needs which part of the context. If an operation needs a frame, it will call `this->frame ()` which will throw if the context does not have a frame. Simon