From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 33304 invoked by alias); 3 Apr 2019 16:20:42 -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 33291 invoked by uid 89); 3 Apr 2019 16:20:41 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.1 spammy=holding, touching X-HELO: mail-wr1-f66.google.com Received: from mail-wr1-f66.google.com (HELO mail-wr1-f66.google.com) (209.85.221.66) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 03 Apr 2019 16:20:40 +0000 Received: by mail-wr1-f66.google.com with SMTP id t17so9171980wrw.13 for ; Wed, 03 Apr 2019 09:20:40 -0700 (PDT) Return-Path: Received: from ?IPv6:2001:8a0:f913:f700:56ee:75ff:fe8d:232b? ([2001:8a0:f913:f700:56ee:75ff:fe8d:232b]) by smtp.gmail.com with ESMTPSA id b134sm37757730wmd.26.2019.04.03.09.20.36 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 03 Apr 2019 09:20:37 -0700 (PDT) Subject: Re: [PATCH v2 15/22] Make exceptions use std::string and be self-managing To: Tom Tromey , gdb-patches@sourceware.org References: <20190227201849.32210-1-tom@tromey.com> <20190227201849.32210-16-tom@tromey.com> From: Pedro Alves Message-ID: Date: Wed, 03 Apr 2019 16:20:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <20190227201849.32210-16-tom@tromey.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2019-04/txt/msg00045.txt.bz2 On 02/27/2019 08:18 PM, Tom Tromey wrote: > This changes the exception's "message" member to be a std::string. > This allows removing the stack of exception messages, because now > exceptions will self-destruct when needed. > > Note that this does increase the cost of copying an exception. This > will be somewhat addressed in a subsequent patch. I have a concern about this patch, something that I alluded to at the Cauldron. I think it'd be better if we followed what the C++ standard library does, and make copy constructor/assignment op nothrow: "Each standard library class T that derives from class exception shall have a publicly accessible copy constructor and a publicly accessible copy assignment operator that do not exit with an exception." (N4659/21.8.2) See: https://stackoverflow.com/questions/21644735/should-i-declare-the-copy-constructor-of-my-exceptions-noexcept I believe that rule exists because throwing an exception copies the exception object, and if that copy operation throws, then the process is terminated with std::terminate(). Copying a std::string may require a heap allocation, which likely doesn't throw on most OSs with over-commit enabled, but it pedantically can. I think fixing this isn't hard, so I think we should do it from the start -- instead of gdb_exception holding a std::string, hold a reference counted immutable heap-allocated C string. > throw_error (except.error, > _("unable to read value of %s (%s)"), > - xvz_name, except.message); > + xvz_name, except.message.c_str ()); Since you're touching most (all?) lines that refer to "message", did you consider adding a "what()" method, to model what std::exception has? const char *what() const noexcept; Maybe that helps with newcomers' familiarity? Thanks, Pedro Alves