From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 120731 invoked by alias); 18 Oct 2016 17:38:49 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 120722 invoked by uid 89); 18 Oct 2016 17:38:48 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.2 required=5.0 tests=BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=smell X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 18 Oct 2016 17:38:38 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 2175480088; Tue, 18 Oct 2016 17:38:37 +0000 (UTC) Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u9IHcZh1013449; Tue, 18 Oct 2016 13:38:36 -0400 Subject: Re: [RFA v2 16/17] Convert dwarf_expr_context_funcs to methods To: Tom Tromey References: <1476393012-29987-1-git-send-email-tom@tromey.com> <1476393012-29987-17-git-send-email-tom@tromey.com> <94f5892b-e4d2-f903-36e2-513e37f3947b@redhat.com> <878ttmid6e.fsf@tromey.com> <4e861038-bc8a-6667-9a11-6c37e7ed4489@redhat.com> <87a8e13gbp.fsf@tromey.com> Cc: gdb-patches@sourceware.org From: Pedro Alves Message-ID: <2d69a217-00ca-a516-8b65-b3d0f0133a03@redhat.com> Date: Tue, 18 Oct 2016 17:38:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-Version: 1.0 In-Reply-To: <87a8e13gbp.fsf@tromey.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-SW-Source: 2016-10/txt/msg00518.txt.bz2 On 10/18/2016 03:03 PM, Tom Tromey wrote: >>>>>> "Pedro" == Pedro Alves writes: > >>> All I saw was the comment about possibly reducing churn: >>> https://sourceware.org/ml/gdb-patches/2016-10/msg00208.html > > Pedro> I'm sorry, I had written comments about the parenthesis > Pedro> thing and also about wondering whether these methods with > Pedro> internal_error could be pure virtual ('= 0;') instead, and if not, > Pedro> about adding a small comment mentioning it. > > I think the internal_error thing can't be a pure virtual because there > are subclasses that do not implement this method. I could push the > internal error into the subclasses though. Hmm. I guess the actual code smell is that these are internal_errors instead of normal errors in the first place? I.e., why are these two cases internal_errors when the rest of the virtual methods have defaults that call normal error instead? Put another way, is there something in the inheritance design that makes calling these default implementations a gdb bug? Cases like "virtual foo() and virtual bar() work in tandem, so if you get to the default bar(), it means you overrode foo() but forgot bar()" would be those which I think would be reasonable to internal_error. I.e., clear implementation bugs. So I'm wondering whether invalid dwarf (i.e., input) or something like that could trigger these internal errors? In any case, I see now that the old code internal_errors as well, so in the name of keeping the patch's scope be converting function pointer tables to virtual methods, it makes sense to keep the internal_errors. So patch is fine as is with the internal_error. Thanks for the patience. Thanks, Pedro Alves