From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 108375 invoked by alias); 8 Nov 2017 09:32:57 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 69547 invoked by uid 89); 8 Nov 2017 09:32:44 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.3 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_NONE,RCVD_IN_SORBS_SPAM,SPF_PASS autolearn=no version=3.3.2 spammy=HX-Received:Wed X-HELO: mail-wr0-f177.google.com Received: from mail-wr0-f177.google.com (HELO mail-wr0-f177.google.com) (209.85.128.177) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 08 Nov 2017 09:32:42 +0000 Received: by mail-wr0-f177.google.com with SMTP id o88so1788994wrb.6 for ; Wed, 08 Nov 2017 01:32:42 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:mail-followup-to:cc:subject:references :date:message-id:user-agent:mime-version; bh=Ru0vJZZaEKeT3PR7rXPmiRIt9KgVpH1b9+HPv4x8z8o=; b=Sqh4Vubkd+a1cOVDdd1OStWXV1u+b/lZc0armqfmHh/GxlzkIPdZQiKCgnGuJYa1ah 8VWF0xrn0RT+FKXQHFakXihQIgSYQYfbuT8Gx1RD/SMVB90/UjlP/kH4oQxBYrJnn2uK UL41FOIcv4XeDcdLrwotJTf9siD/7p3erbRz8LU1t6iW0s7Jx6l3VB0giFW8D6L/JRVd utLW6PBVhDJ8yqkGXuAevpfRQeYITkexxl8ujYbdk3K0R/NFrHrvUbny4oKljrv6aKCe GdJBpJ+ny7Ni6I3428GAHJBN4FiV45NDWFE+Z4LIV24/v8eusLhwPTe8DvVih9Wsg56U slEA== X-Gm-Message-State: AJaThX7TIBUj3fiCl1235FhmUNTYEQqYRKA7uEs5movxmeU8tckTXeek 3JpZRU+ruHbQJ+GRkLDTVzz6GYWtrsc= X-Google-Smtp-Source: ABhQp+T7lx+/faMMzKWOvdtYiopxJ6xGItqU2bC41VqcJVSPGAjbx4lENGzUknKtFum0pSahbjIvbg== X-Received: by 10.223.134.157 with SMTP id 29mr1309644wrx.72.1510133560064; Wed, 08 Nov 2017 01:32:40 -0800 (PST) Received: from localhost ([2.25.234.120]) by smtp.gmail.com with ESMTPSA id d4sm2899696wma.29.2017.11.08.01.32.39 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Wed, 08 Nov 2017 01:32:39 -0800 (PST) From: Richard Sandiford To: Martin Sebor Mail-Followup-To: Martin Sebor ,gcc-patches@gcc.gnu.org, richard.sandiford@linaro.org Cc: gcc-patches@gcc.gnu.org Subject: Re: [001/nnn] poly_int: add poly-int.h References: <871sltvm7r.fsf@linaro.org> <87vaj5u7id.fsf@linaro.org> Date: Wed, 08 Nov 2017 09:44:00 -0000 Message-ID: <87po8t5d55.fsf@linaro.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.2 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-SW-Source: 2017-11/txt/msg00563.txt.bz2 Martin Sebor writes: > I haven't done nearly a thorough review but the dtor followed by > the placement new in the POLY_SET_COEFF() macro caught my eye so > I thought I'd ask sooner rather than later. Given the macro > definition: > > + The dummy comparison against a null C * is just a way of checking > + that C gives the right type. */ > +#define POLY_SET_COEFF(C, RES, I, VALUE) \ > + ((void) (&(RES).coeffs[0] == (C *) 0), \ > + wi::int_traits::precision_type == wi::FLEXIBLE_PRECISION \ > + ? (void) ((RES).coeffs[I] = VALUE) \ > + : (void) ((RES).coeffs[I].~C (), new (&(RES).coeffs[I]) C (VALUE))) > > is the following use well-defined? > > +template > +inline poly_int_pod& > +poly_int_pod::operator <<= (unsigned int a) > +{ > + POLY_SET_COEFF (C, *this, 0, this->coeffs[0] << a); > > It looks to me as though the VALUE argument in the ctor invoked > by the placement new expression is evaluated after the dtor has > destroyed the very array element the VALUE argument expands to. Good catch! It should simply have been doing <<= on each coefficient -- I must have got carried away when converting to POLY_SET_COEFF. I double-checked the other uses and think that's the only one. > Whether or not is, in fact, a problem, it seems to me that using > a function template rather than a macro would be a clearer and > safer way to do the same thing. (Safer in that the macro also > evaluates its arguments multiple times, which is often a source > of subtle bugs.) That would slow down -O0 builds though, by introducing an extra function call and set of temporaries even when the coefficients are primitive integers. > Other than that, I would suggest changing 't' to something a bit > less terse, like perhaps 'type' in traits like the following: > > +struct if_lossless; > +template > +struct if_lossless > +{ > + typedef T3 t; > +}; OK, done in v2. Thanks, Richard