From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 89572 invoked by alias); 18 Jul 2019 16:07:23 -0000 Mailing-List: contact jit-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Post: List-Help: List-Subscribe: Sender: jit-owner@gcc.gnu.org Received: (qmail 89554 invoked by uid 89); 18 Jul 2019 16:07:22 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Checked: by ClamAV 0.100.3 on sourceware.org X-Virus-Found: No X-Spam-SWARE-Status: No, score=-19.0 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,SPF_HELO_PASS autolearn=ham version=3.3.1 spammy=sk:unary_o, H*i:sk:gkr36j3, H*f:sk:gkr36j3 X-Spam-Status: No, score=-19.0 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,SPF_HELO_PASS autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on sourceware.org X-Spam-Level: X-Spam-User: qpsmtpd, 2 recipients 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; Thu, 18 Jul 2019 16:07:21 +0000 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 761633082145; Thu, 18 Jul 2019 16:07:20 +0000 (UTC) Received: from ovpn-117-34.phx2.redhat.com (ovpn-117-34.phx2.redhat.com [10.3.117.34]) by smtp.corp.redhat.com (Postfix) with ESMTP id C5D735C220; Thu, 18 Jul 2019 16:07:19 +0000 (UTC) Message-ID: <1563466038.2530.92.camel@redhat.com> Subject: Re: [PATCH][gcc] libgccjit: check result_type in gcc_jit_context_new_unary_op From: David Malcolm To: Andrea Corallo , "jit@gcc.gnu.org" , "gcc-patches@gcc.gnu.org" Cc: nd Date: Tue, 01 Jan 2019 00:00:00 -0000 In-Reply-To: References: Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 7bit X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.42]); Thu, 18 Jul 2019 16:07:20 +0000 (UTC) X-IsSubscribed: yes X-SW-Source: 2019-q3/txt/msg00006.txt.bz2 On Thu, 2019-07-18 at 14:20 +0000, Andrea Corallo wrote: > Hi all, > I've just realized that what we has been done recently for > gcc_jit_context_new_binary_op should be done also for the unary > version. > This patch checks at record time for the result type of > gcc_jit_context_new_unary_op to be numeric type plus add a testcase > for the new check. > > make check-jit runs clean > > Is it okay for trunk? > > Bests > Andrea > > gcc/jit/ChangeLog > 2019-07-18 Andrea Corallo > > * libgccjit.c (gcc_jit_context_new_unary_op): Check result_type > to be a > numeric type. > * libgccjit.c (gcc_jit_context_new_binary_op): Fix nit in error > message. > > gcc/testsuite/ChangeLog > 2019-07-04 Andrea Corallo > > * jit.dg/test-error-gcc_jit_context_new_unary_op-bad-res- > type.c: > New testcase. > * jit.dg/test-error-gcc_jit_context_new_binary_op-bad-res- > type.c: > Fix nit in error message. Thanks for the patch. What happens with the existing code if the user tries to use such a unary op? > diff --git a/gcc/jit/libgccjit.c b/gcc/jit/libgccjit.c > index 23e83e2..bea840f 100644 > --- a/gcc/jit/libgccjit.c > +++ b/gcc/jit/libgccjit.c > @@ -1336,6 +1336,12 @@ gcc_jit_context_new_unary_op (gcc_jit_context *ctxt, > "unrecognized value for enum gcc_jit_unary_op: %i", > op); > RETURN_NULL_IF_FAIL (result_type, ctxt, loc, "NULL result_type"); > + RETURN_NULL_IF_FAIL_PRINTF3 ( > + result_type->is_numeric (), ctxt, loc, > + "gcc_jit_unary_op %i with operand %s " > + "has non-numeric result_type: %s", > + op, rvalue->get_debug_string (), > + result_type->get_debug_string ()); > RETURN_NULL_IF_FAIL (rvalue, ctxt, loc, "NULL rvalue"); The use of "%i" for "op" here isn't as user-friendly as it could be; it would be ideal to tell the user the enum value. "op" has already been validated, so why not expose the currently-static unary_op_reproducer_strings from jit-recording.c in an internal header, and use it here with a "%s"? > return (gcc_jit_rvalue *)ctxt->new_unary_op (loc, op, result_type, rvalue); > @@ -1388,7 +1394,7 @@ gcc_jit_context_new_binary_op (gcc_jit_context *ctxt, > RETURN_NULL_IF_FAIL_PRINTF4 ( > result_type->is_numeric (), ctxt, loc, > "gcc_jit_binary_op %i with operands a: %s b: %s " > - "has non numeric result_type: %s", > + "has non-numeric result_type: %s", > op, a->get_debug_string (), b->get_debug_string (), > result_type->get_debug_string ()); Ah, I see there's one of these "%i" for op already. Given that you're already fixing a nit here, please make this print "%s", using binary_op_reproducer_strings from jit-recording.c ("op" has already been validated). Thanks Dave