From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id B96D33858024 for ; Tue, 6 Sep 2022 11:27:56 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org B96D33858024 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1662463676; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=lfg3nCP/LKgVhQSQwTxFuTC/ntv1yPZUnXMQQ5V/xXU=; b=Kj0oWLndkfzriuDnt+TOxI6TQ3VdNdMykk7uA6aKNYVWABYv1aAoDreHlaFr9glnv6cfBc teB0f8O2jrMqm2O/we4YXokFZYl0XGOyZ2SpIig8jaPfpj46uTckvaxrK3ZpMWNte9Xu5s +doJbugqPWP3Of6g8J2SgHlT3yxvRj0= Received: from mail-qk1-f198.google.com (mail-qk1-f198.google.com [209.85.222.198]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-226-0E1FiKBONq-VdWgsiLSItw-1; Tue, 06 Sep 2022 07:27:55 -0400 X-MC-Unique: 0E1FiKBONq-VdWgsiLSItw-1 Received: by mail-qk1-f198.google.com with SMTP id s9-20020a05620a254900b006b54dd4d6deso8789758qko.3 for ; Tue, 06 Sep 2022 04:27:55 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date; bh=lfg3nCP/LKgVhQSQwTxFuTC/ntv1yPZUnXMQQ5V/xXU=; b=Jta6/LD2j7CZ1rcOK3vLMUMIOUkgGr3+JmACjD4E7zHMrUo0SjaYlYhwCrMuVUp1Ds fw+IoCzx6BLbFyM0uj8pOKmnYJ2fhn1l8SNqBsTrMU3qg+CYjOqA9v5prGovgTDP7poy BdL+yhk7cbc/wZoN7lMEoRMRKb3dQbolFLqDQEEzH5vvFCN73KRKLYQggLuamf4bFgDA R8mmbu40Olz/2yxomllWrIBRe1YVC84+bG1pmUkfBTi6wxeVbO2r1zVvCyyju2reUSS4 aGrZiD07aqRJdyQFunH///33SlVPR8KdCAq2LnpFqKXePwqPniOcgwBo/wPveupoTntR mOWw== X-Gm-Message-State: ACgBeo0gyIsmccGKREhtj2WHOSXL8MF51HPPF9j8GYSmQHCvTp95+m7Q +WiSwkgRURyUTQK4qV3FYlNiDTD7KQvyUlwCUjVPjXybz2mkxp+u2X+3HuFL3J6lZRdEW3QsF70 Gn65synUUDRqImQxe/mFweKEGEKTOR3c= X-Received: by 2002:a05:620a:2728:b0:6ba:e857:9375 with SMTP id b40-20020a05620a272800b006bae8579375mr34923222qkp.728.1662463674702; Tue, 06 Sep 2022 04:27:54 -0700 (PDT) X-Google-Smtp-Source: AA6agR7LlFuzlFwE+0f2ar3PrdfVE4vpOKfFpIfpcMvGKMsAc6zwfLHzsCsonxvscH4AgkR/9gy6+VTWu21kg4AMxAc= X-Received: by 2002:a05:620a:2728:b0:6ba:e857:9375 with SMTP id b40-20020a05620a272800b006bae8579375mr34923212qkp.728.1662463674454; Tue, 06 Sep 2022 04:27:54 -0700 (PDT) MIME-Version: 1.0 References: <20220904184735.177348-1-fent@in.tum.de> <20220904184735.177348-2-fent@in.tum.de> In-Reply-To: <20220904184735.177348-2-fent@in.tum.de> From: Jonathan Wakely Date: Tue, 6 Sep 2022 12:27:43 +0100 Message-ID: Subject: Re: [PATCH 2/2] libstdc++: Add pretty printer for std::stringstream To: Philipp Fent Cc: libstdc++@gcc.gnu.org, gcc-patches@gcc.gnu.org X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-12.5 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_NONE,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Thanks for the patch. Comments below. On Sun, 4 Sept 2022 at 19:48, Philipp Fent via Libstdc++ wrote: > > Signed-off-by: Philipp Fent > --- > libstdc++-v3/python/libstdcxx/v6/printers.py | 37 +++++++++++++++++++ > .../libstdc++-prettyprinters/debug.cc | 5 +++ > .../libstdc++-prettyprinters/simple.cc | 5 +++ > .../libstdc++-prettyprinters/simple11.cc | 5 +++ > 4 files changed, 52 insertions(+) > > diff --git a/libstdc++-v3/python/libstdcxx/v6/printers.py b/libstdc++-v3/python/libstdcxx/v6/printers.py > index d70c8d5d616..5083f693387 100644 > --- a/libstdc++-v3/python/libstdcxx/v6/printers.py > +++ b/libstdc++-v3/python/libstdcxx/v6/printers.py > @@ -969,6 +969,39 @@ class StdStringPrinter: > def display_hint (self): > return 'string' > > +class StdStringBufPrinter: > + "Print a std::basic_stringbuf" > + > + def __init__(self, _, val): > + self.val = val > + > + def to_string(self): > + pbase = self.val['_M_out_beg'] > + pptr = self.val['_M_out_cur'] > + egptr = self.val['_M_in_end'] It would be nice to encapsulate this into a generic helper for all streambufs, as this part isn't specific to basic_stringbuf. We could then reuse it for printing spanbufs and other types in future. It could be a function that takes a streambuf and returns a tuple of (pbase, pptr, egptr). > + # Logic from basic_stringbuf::_M_high_mark() > + if pptr: > + if not egptr or pptr > egptr: > + return pbase.string(length = pptr - pbase) > + else: > + return pbase.string(length = pptr - egptr) Shouldn't this be egptr - pbase? This suggests the tests are inadequate. I think this bug would have been found by a test something like: std::stringstream ssin("input", std::ios::in); // { dg-final { note-test ssin "\"input\"" } } The (egptr - pbase) case is also needed for printing an istringstream. > + return self.val['_M_string'] > + > + def display_hint(self): > + return 'string' > + > +class StdStringStreamPrinter: > + "Print a std::basic_stringstream" > + > + def __init__(self, _, val): > + self.val = val > + > + def to_string(self): > + return self.val['_M_stringbuf'] This assumes that the stream is still using its stringbuf, which is probably true 99% of the time, but isn't guaranteed. In the following situation, the printer would give potentially misleading output: std::stringstream ss("xxx"); ss.rdbuf(std::cin.rdbuf()); I wonder if we want to have a check that self.val['_M_streambuf'] == self.val['_M_stringbuf'].address Alternatively, don't provide a printer for the stringstream at all, just for the stringbuf, and then when GDB uses its default display for the stringstream it will show that member. > + > + def display_hint(self): > + return 'string' > + > class Tr1HashtableIterator(Iterator): > def __init__ (self, hashtable): > self.buckets = hashtable['_M_buckets'] > @@ -2232,6 +2265,10 @@ def build_libstdcxx_dictionary (): > libstdcxx_printer.add_version('std::', 'initializer_list', > StdInitializerListPrinter) > libstdcxx_printer.add_version('std::', 'atomic', StdAtomicPrinter) > + libstdcxx_printer.add_version('std::', 'basic_stringbuf', StdStringBufPrinter) > + libstdcxx_printer.add_version('std::__cxx11::', 'basic_stringbuf', StdStringBufPrinter) > + libstdcxx_printer.add_version('std::', 'basic_stringstream', StdStringStreamPrinter) > + libstdcxx_printer.add_version('std::__cxx11::', 'basic_stringstream', StdStringStreamPrinter) Wouldn't we want it for ostringstream and istringstream too?