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 ESMTP id 3A417395C408 for ; Thu, 27 May 2021 22:19:55 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 3A417395C408 Received: from mail-qv1-f72.google.com (mail-qv1-f72.google.com [209.85.219.72]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-28-H64Bx8BjOs6a1eZbCJMTPg-1; Thu, 27 May 2021 18:19:53 -0400 X-MC-Unique: H64Bx8BjOs6a1eZbCJMTPg-1 Received: by mail-qv1-f72.google.com with SMTP id l16-20020ad442500000b029021344b553c5so1230311qvq.5 for ; Thu, 27 May 2021 15:19:53 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:message-id:subject:from:to:cc:date:in-reply-to :references:user-agent:mime-version:content-transfer-encoding; bh=ApjT33hvTVAFJ9qMyrj/B4gIKnOxnqnmITioMtmHfyE=; b=JZIwo2jcGyxZXWQKeD1IlXigkrlD5pK9dXX5lvmlMnMruVlQ5bZqGLIEFbgbk6NBcz n8BMhpDdbHgG5k9PDESePE3WsAKOPJJeTdaOrb8ciU2M2vTp0sGm7MXmO/08sDTq0Sj1 lrB7gYp2NIiSLV2WGl6mzAPEBBg9NJqPSHqARyACH4i+SZOzou621WWMpnXqawhJMqMo ZFttsyFcrqWiAJUZN/9YA6Z5an39K8p8hSGeml77cdRtBp7sKC7PNhNQTWYZ0PDpMbQD 04QCR8mCfvt9S4eNbm/ITwNTTn1aC6y00/0siM9oMg1ffXWPBG54HFAIwSqB/tyMGtkr UsNg== X-Gm-Message-State: AOAM533FUpu9E+5v7m3wz2WOWnpjykzICSW56vWDbbA53BveDZDBPcGL 3f+ZWKJMreB1A4cL52CntZoNZUBiZxtOVl43B2kQ9tnKDUia4FEBwJVP3Tb7bBOP1Koq4Vy49ZF Yg9tCMiI= X-Received: by 2002:a05:620a:a5a:: with SMTP id j26mr803917qka.156.1622153992783; Thu, 27 May 2021 15:19:52 -0700 (PDT) X-Google-Smtp-Source: ABdhPJyk7GICZHDK3R6Q4HJvFJcoXER7JS/vN1juJEh0A4S/ZoZ6o/kj1yWf3QGq7Q/S1ncrfGOZiw== X-Received: by 2002:a05:620a:a5a:: with SMTP id j26mr803887qka.156.1622153992295; Thu, 27 May 2021 15:19:52 -0700 (PDT) Received: from t14s.localdomain (c-73-69-212-193.hsd1.ma.comcast.net. [73.69.212.193]) by smtp.gmail.com with ESMTPSA id t25sm2163729qtp.84.2021.05.27.15.19.50 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 27 May 2021 15:19:51 -0700 (PDT) Message-ID: <534254132a841b75d555a52ce952f84418f168c2.camel@redhat.com> Subject: Re: [PATCH] libgccjit: add some reflection functions in the jit C api From: David Malcolm To: Antoni Boucher Cc: Andrea Corallo , Antoni Boucher via Jit , gcc-patches@gcc.gnu.org Date: Thu, 27 May 2021 18:19:49 -0400 In-Reply-To: <0e8b6450bcb23182b85342d8010c3bea0c297ba2.camel@zoho.com> References: <20200902010120.crnx55ev635ceiey@bouanto-desktop.localdomain> <6fe2ea355403eb177c9632e076c11c792f23c791.camel@redhat.com> <9cd00fe5cb5c03184e3a5bd939447b30450a8ca7.camel@redhat.com> <20201015160226.cka4iq3w22jztopm@bouanto-desktop.localdomain> <20201015173952.ft7mu5ajbaxb3gke@bouanto-desktop.localdomain> <54ba5c58dbd2b8c7b1f3276c2b87cddf55e258bd.camel@redhat.com> <20201103221324.xw3jbvrw2uwdc4rz@bouanto-desktop.localdomain> <3388bb8c84e68cd7b0698dc154e7a5666c0d2cde.camel@redhat.com> <0e8b6450bcb23182b85342d8010c3bea0c297ba2.camel@zoho.com> User-Agent: Evolution 3.38.4 (3.38.4-1.fc33) MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-10.8 required=5.0 tests=BAYES_00, BODY_8BITS, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: jit@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Jit mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 27 May 2021 22:19:57 -0000 On Tue, 2021-05-25 at 20:19 -0400, Antoni Boucher wrote: > @David: PING > > As far as I know, the only remaining question is about using > `ssize_t` > for the return type of some functions. > Here's why I use this type: > > That seemed off to return NULL for the functions returning a > size_t to indicate an error. So I changed it to return -1 (and return > type to ssize_t). Is that the proper way to indicate an error? > > Once I know the answer for this error handling question, I'll fix the > types. > > Thanks! I think the things that accept params for indexes should be size_t (i.e. unsigned). As for the error-handling, I've been going back and forth. The various return of -1 in the patch all occur when NULL is passed in as the gcc_jit_foo*, for the various foo. Unfortunately this means that we can't get at a jit context to emit the error on, so jit_error is called with a NULL context, and hence merely prints to stderr; it can't set state on any jit context. Presumably if the code is passing in NULL for one of these things, then something's gone wrong already. Option (A): returning a size_t (unsigned): - No way to indicate error if the user passes in NULL, or a bogus value, beyond output on stderr; we have to defensively return 0, which the user's code is likely to gracefully handle Option (B): returning a ssize_t: - Can return -1 if the user passes in NULL or a bogus value. But if the user doesn't check for -1 and blithely casts to unsigned (e.g. allocating a buffer), their code is likely to explode with a very large allocation. Consider e.g. gcc_jit_function_type_get_param_count. If the user uses  gcc_jit_type_is_function_ptr_type to dynamically cast some type to function_type and forgets to check, and passes in NULL, then... ...with option (A), it returns 0, as if the function has no params; eventually a type-checking error occurs. ...with option (B), it returns -1, and the user who didn't check before has to check again So I think I prefer option (A), but I'm willing to be convinced on the alternative. I also now think that the dynamic cast functions (e.g. gcc_jit_type_is_function_ptr_type) should be "_dyncast_" rather than "_is_" (e.g. gcc_jit_type_dyncast_function_ptr_type) to make it clear that this is a dynamic cast that can fail, and to contrast them with the "_as_" functions which are static casts. Hope this makes sense and is constructive. Dave > > Le jeudi 13 mai 2021 à 17:30 -0400, David Malcolm a écrit : > > On Tue, 2020-11-03 at 17:13 -0500, Antoni Boucher wrote: > > > I was missing a check in gcc_jit_struct_get_field, I added it in > > > this > > > new patch. > > > > > > > Sorry about the long delay in reviewing this patch. > > > > The main high-level points are: > > - currently the get_*_count functions return "ssize_t" - why?  Only > > unsigned values are meaningful; shouldn't they return "size_t" > > instead? > > > > - the various "lookup by index" functions take "int" i.e. signed, but > > only >= 0 is meaningful.  I think it makes sense to make them take > > size_t instead. > > > > Sorry if we covered that before in the review, it's been a while. > > > > Various nitpicks inline below... > > > > [...snip...] > >   > > > diff --git a/gcc/jit/docs/topics/compatibility.rst > > > b/gcc/jit/docs/topics/compatibility.rst > > > index 6bfa101ed71..236e5c72d81 100644 > > > --- a/gcc/jit/docs/topics/compatibility.rst > > > +++ b/gcc/jit/docs/topics/compatibility.rst > > > @@ -226,3 +226,44 @@ entrypoints: > > >  -------------------- > > >  ``LIBGCCJIT_ABI_14`` covers the addition of > > >  :func:`gcc_jit_global_set_initializer` > > > + > > > +.. _LIBGCCJIT_ABI_15: > > > + > > > +``LIBGCCJIT_ABI_15`` > > > +-------------------- > > > +``LIBGCCJIT_ABI_15`` covers the addition of reflection functions > > > via > > > API > > > +entrypoints: > > > > This needs updating, as I used LIBGCCJIT_ABI_15 for inline asm. > > > > [...snip...] > > > > > diff --git a/gcc/jit/docs/topics/functions.rst > > > b/gcc/jit/docs/topics/functions.rst > > > index eb40d64010e..aa6de87282d 100644 > > > --- a/gcc/jit/docs/topics/functions.rst > > > +++ b/gcc/jit/docs/topics/functions.rst > > > @@ -171,6 +171,16 @@ Functions > > >     underlying string, so it is valid to pass in a pointer to an > > > on- > > > stack > > >     buffer. > > >   > > > +.. function::  ssize_t \ > > > +               gcc_jit_function_get_param_count (gcc_jit_function > > > *func) > > > + > > > +   Get the number of parameters of the function. > > > + > > > +.. function::  gcc_jit_type \* > > > +               gcc_jit_function_get_return_type (gcc_jit_function > > > *func) > > > + > > > +   Get the return type of the function. > > > > As noted before, this doesn't yet document all the new entrypoints; I > > think you wanted to hold off until all the details were thrashed out, > > but hopefully we're close. > > > > The documentation for an entrypoint should specify which ABI it was > > added in. > > > > [...snip...] > > > > > +/* Public entrypoint.  See description in libgccjit.h. > > > + > > > +   After error-checking, the real work is done by the > > > +   gcc::jit::recording::type::is_struct method, in > > > +   jit-recording.c.  */ > > > + > > > +gcc_jit_struct * > > > +gcc_jit_type_is_struct (gcc_jit_type *type) > > > +{ > > > +  RETURN_NULL_IF_FAIL (type, NULL, NULL, "NULL type"); > > > +  gcc::jit::recording::struct_ *struct_type = type->is_struct (); > > > +  return (gcc_jit_struct *)struct_type; > > > +} > > > + > > > +/* Public entrypoint.  See description in libgccjit.h. > > > + > > > +   After error-checking, the real work is done by the > > > +   gcc::jit::recording::vector_type::get_num_units method, in > > > +   jit-recording.c.  */ > > > + > > > +ssize_t > > > +gcc_jit_vector_type_get_num_units (gcc_jit_vector_type > > > *vector_type) > > > +{ > > > +  RETURN_VAL_IF_FAIL (vector_type, -1, NULL, NULL, "NULL > > > vector_type"); > > > +  return vector_type->get_num_units (); > > > +} > > > + > > > +/* Public entrypoint.  See description in libgccjit.h. > > > + > > > +   After error-checking, the real work is done by the > > > +   gcc::jit::recording::vector_type::get_element_type method, in > > > +   jit-recording.c.  */ > > > + > > > +gcc_jit_type * > > > +gcc_jit_vector_type_get_element_type (gcc_jit_vector_type > > > *vector_type) > > > +{ > > > +  RETURN_NULL_IF_FAIL (vector_type, NULL, NULL, "NULL > > > vector_type"); > > > +  return (gcc_jit_type *)vector_type->get_element_type (); > > > +} > > > + > > > +/* Public entrypoint.  See description in libgccjit.h. > > > + > > > +   After error-checking, the real work is done by the > > > +   gcc::jit::recording::type::unqualified method, in > > > +   jit-recording.c.  */ > > > + > > > +gcc_jit_type * > > > +gcc_jit_type_unqualified (gcc_jit_type *type) > > > +{ > > > +  RETURN_NULL_IF_FAIL (type, NULL, NULL, "NULL type"); > > > + > > > +  return (gcc_jit_type *)type->unqualified (); > > > +} > > > + > > > +/* Public entrypoint.  See description in libgccjit.h. > > > + > > > +   After error-checking, the real work is done by the > > > +   gcc::jit::recording::type::dyn_cast_function_type method, in > > > +   jit-recording.c.  */ > > > + > > > +gcc_jit_function_type * > > > +gcc_jit_type_is_function_ptr_type (gcc_jit_type *type) > > > +{ > > > +  RETURN_NULL_IF_FAIL (type, NULL, NULL, "NULL type"); > > > +  gcc::jit::recording::type *func_ptr_type = type->dereference (); > > > +  RETURN_NULL_IF_FAIL (func_ptr_type, NULL, NULL, "NULL type"); > > > +  gcc::jit::recording::function_type *func_type = > > > +    func_ptr_type->dyn_cast_function_type (); > > > +  RETURN_NULL_IF_FAIL (func_type, NULL, NULL, "NULL type"); > > > > I notice that the other new "*_is_*" functions don't fail if the > > dyncast returns NULL, whereas this one does. > > > > RETURN_NULL_IF_FAIL calls jit_error; do we really want that?  It > > seems > > more consistent to have it return NULL without an error for the case > > where "type" isn't a function ptr type. > > > > > + > > > +  return (gcc_jit_function_type *)func_type; > > > +} > > > + > > > +/* Public entrypoint.  See description in libgccjit.h. > > > + > > > +   After error-checking, the real work is done by the > > > +   gcc::jit::recording::function_type::get_return_type method, in > > > +   jit-recording.c.  */ > > > + > > > +gcc_jit_type * > > > +gcc_jit_function_type_get_return_type (gcc_jit_function_type > > > *function_type) > > > +{ > > > +  RETURN_NULL_IF_FAIL (function_type, NULL, NULL, "NULL > > > struct_type"); > > > > Wrong error message; should be "NULL function_type". > > > > > > > +  return (gcc_jit_type *)function_type->get_return_type (); > > > +} > > > + > > > +/* Public entrypoint.  See description in libgccjit.h. > > > + > > > +   After error-checking, the real work is done by the > > > +   gcc::jit::recording::function_type::get_param_types method, in > > > +   jit-recording.c.  */ > > > + > > > +ssize_t > > > +gcc_jit_function_type_get_param_count (gcc_jit_function_type > > > *function_type) > > > +{ > > > +  RETURN_VAL_IF_FAIL (function_type, -1, NULL, NULL, "NULL > > > struct_type"); > > > > Wrong error message; should be "NULL function_type". > > > > > +  return function_type->get_param_types ().length (); > > > +} > > > + > > > +/* Public entrypoint.  See description in libgccjit.h. > > > + > > > +   After error-checking, the real work is done by the > > > +   gcc::jit::recording::function_type::get_param_types method, in > > > +   jit-recording.c.  */ > > > + > > > +gcc_jit_type * > > > +gcc_jit_function_type_get_param_type (gcc_jit_function_type > > > *function_type, > > > +                               int index) > > > +{ > > > +  RETURN_NULL_IF_FAIL (function_type, NULL, NULL, "NULL > > > struct_type"); > > > > Wrong error message; should be "NULL function_type". > > > > > > > +  int num_params = function_type->get_param_types ().length (); > > > +  gcc::jit::recording::context *ctxt = function_type->m_ctxt; > > > > "index" should either be unsigned, or we need to check here for >= 0. > > > > > +  RETURN_NULL_IF_FAIL_PRINTF3 (index < num_params, > > > +                              ctxt, NULL, > > > +                              "index of %d is too large (%s has %d > > > params)", > > > +                              index, > > > +                              function_type->get_debug_string (), > > > +                              num_params); > > > +  return (gcc_jit_type *)function_type->get_param_types ()[index]; > > > +} > > > + > > >  /* Public entrypoint.  See description in libgccjit.h. > > >   > > >     After error-checking, the real work is done by the > > > @@ -731,6 +930,36 @@ gcc_jit_struct_set_fields (gcc_jit_struct > > > *struct_type, > > >                            (gcc::jit::recording::field **)fields); > > >  } > > >   > > > + > > > +/* Public entrypoint.  See description in libgccjit.h. > > > + > > > +   After error-checking, the real work is done by the > > > +   gcc::jit::recording::fields::get_field method in > > > +   jit-recording.c.  */ > > > +extern gcc_jit_field * > > > +gcc_jit_struct_get_field (gcc_jit_struct *struct_type, > > > +                          int index) > > > +{ > > > +  RETURN_NULL_IF_FAIL (struct_type, NULL, NULL, "NULL struct > > > type"); > > > +  RETURN_NULL_IF_FAIL (struct_type->get_fields (), NULL, NULL, > > > "NULL > > > struct fields"); > > > > "index" should either be unsigned, or we need to check here for >= 0. > > > > > +  RETURN_NULL_IF_FAIL (index < struct_type->get_fields ()->length > > > (), NULL, > > > +                               NULL, "NULL struct type"); > > > +  return (gcc_jit_field *)struct_type->get_fields ()->get_field > > > (index); > > > +} > > > > [...snip...] > > > > > diff --git a/gcc/jit/libgccjit.h b/gcc/jit/libgccjit.h > > > index 7134841bb07..db4ccffb41c 100644 > > > --- a/gcc/jit/libgccjit.h > > > +++ b/gcc/jit/libgccjit.h > > > @@ -96,6 +96,12 @@ typedef struct gcc_jit_field gcc_jit_field; > > >     the layout for, or an opaque type.  */ > > >  typedef struct gcc_jit_struct gcc_jit_struct; > > >   > > > +/* A gcc_jit_function_type encapsulates a function type.  */ > > > +typedef struct gcc_jit_function_type gcc_jit_function_type; > > > + > > > +/* A gcc_jit_vector_type encapsulates a vector type.  */ > > > +typedef struct gcc_jit_vector_type gcc_jit_vector_type; > > > + > > > > Please add these to the ASCII art inheritance diagram immediately > > above: > >   typedef struct gcc_jit_object gcc_jit_object; > > > > [...snip...] > > > > >  /* Unions work similarly to structs.  */ > > >  extern gcc_jit_type * > > >  gcc_jit_context_new_union_type (gcc_jit_context *ctxt, > > > @@ -1518,6 +1533,78 @@ gcc_jit_version_minor (void); > > >  extern int > > >  gcc_jit_version_patchlevel (void); > > >   > > > +#define LIBGCCJIT_HAVE_REFLECTION > > > + > > > +/* Reflection functions to get the number of parameters, return > > > type > > > of > > > +   a function and whether a type is a bool from the C API. > > > + > > > +   This API entrypoint was added in LIBGCCJIT_ABI_15; you can test > > > for its > > > +   presence using > > > +     #ifdef LIBGCCJIT_HAVE_REFLECTION > > > +*/ > > > +/* Get the return type of a function.  */ > > > +extern gcc_jit_type * > > > +gcc_jit_function_get_return_type (gcc_jit_function *func); > > > + > > > +/* Get the number of params of a function.  */ > > > +extern ssize_t > > > +gcc_jit_function_get_param_count (gcc_jit_function *func); > > > > You're using ssize_t here, why?  Wouldn't size_t be better? > > The count can't be negative. > > > > Should we use size_t for indexes, rather than "int"? > > > > (this happens in various places e.g. > > gcc_jit_function_type_get_param_count and > > gcc_jit_vector_type_get_num_units) > > > > [...snip...] > > > > > diff --git a/gcc/jit/libgccjit.map b/gcc/jit/libgccjit.map > > > index a6e67e781a4..720b6ca286f 100644 > > > --- a/gcc/jit/libgccjit.map > > > +++ b/gcc/jit/libgccjit.map > > > @@ -192,3 +192,24 @@ LIBGCCJIT_ABI_14 { > > >    global: > > >      gcc_jit_global_set_initializer; > > >  } LIBGCCJIT_ABI_13; > > > + > > > +LIBGCCJIT_ABI_15 { > > > +  global: > > > +    gcc_jit_function_get_return_type; > > > +    gcc_jit_function_get_param_count; > > > +    gcc_jit_function_type_get_return_type; > > > +    gcc_jit_function_type_get_param_count; > > > +    gcc_jit_function_type_get_param_type; > > > +    gcc_jit_type_unqualified; > > > +    gcc_jit_type_is_array; > > > +    gcc_jit_type_is_bool; > > > +    gcc_jit_type_is_function_ptr_type; > > > +    gcc_jit_type_is_integral; > > > +    gcc_jit_type_is_pointer; > > > +    gcc_jit_type_is_vector; > > > +    gcc_jit_vector_type_get_element_type; > > > +    gcc_jit_vector_type_get_num_units; > > > +    gcc_jit_struct_get_field; > > > +    gcc_jit_type_is_struct; > > > +    gcc_jit_struct_get_field_count; > > > +} LIBGCCJIT_ABI_14; > > > > As noted above, this will need to be LIBGCCJIT_ABI_16 as I used > > LIBGCCJIT_ABI_15 for inline asm. > > > > [...snip...] > > > > > diff --git a/gcc/testsuite/jit.dg/test-reflection.c > > > b/gcc/testsuite/jit.dg/test-reflection.c > > > new file mode 100644 > > > index 00000000000..b3cf2942f3a > > > --- /dev/null > > > +++ b/gcc/testsuite/jit.dg/test-reflection.c > > > @@ -0,0 +1,89 @@ > > > +#include > > > +#include > > > + > > > +#include "libgccjit.h" > > > + > > > +#include "harness.h" > > > + > > > +void > > > +create_code (gcc_jit_context *ctxt, void *user_data) > > > +{ > > > +  /* Do nothing.  */ > > > +} > > > + > > > +void > > > +verify_code (gcc_jit_context *ctxt, gcc_jit_result *result) > > > +{ > > > +  /* Get the built-in functions.  */ > > > +  gcc_jit_function *builtin_sin = > > > +    gcc_jit_context_get_builtin_function (ctxt, "sin"); > > > + > > > +  CHECK_VALUE (gcc_jit_function_get_param_count(builtin_sin), 1); > > > + > > > +  gcc_jit_type *double_type = > > > +    gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_DOUBLE); > > > +  CHECK_VALUE (gcc_jit_function_get_return_type(builtin_sin), > > > double_type); > > > +  CHECK (!gcc_jit_type_is_integral(double_type)); > > > + > > > +  gcc_jit_type *bool_type = > > > +    gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_BOOL); > > > +  CHECK (gcc_jit_type_is_bool(bool_type)); > > > +  CHECK (!gcc_jit_type_is_integral(bool_type)); > > > + > > > +  gcc_jit_type *aligned_bool_type = > > > +    gcc_jit_type_get_aligned(gcc_jit_context_get_type (ctxt, > > > GCC_JIT_TYPE_BOOL), 8); > > > +  CHECK (gcc_jit_type_is_bool(aligned_bool_type)); > > > +  CHECK (bool_type != aligned_bool_type); > > > +  CHECK_VALUE (gcc_jit_type_unqualified(aligned_bool_type), > > > bool_type); > > > + > > > +  CHECK_VALUE > > > (gcc_jit_type_unqualified(gcc_jit_type_get_const(bool_type)), > > > bool_type); > > > +  CHECK_VALUE > > > (gcc_jit_type_unqualified(gcc_jit_type_get_volatile(bool_type)), > > > bool_type); > > > + > > > +  gcc_jit_type *int64 = > > > +    gcc_jit_context_get_int_type(ctxt, 8, 1); > > > +  CHECK (gcc_jit_type_is_integral(int64)); > > > +  gcc_jit_type *uint64 = > > > +    gcc_jit_context_get_int_type(ctxt, 8, 0); > > > +  CHECK (gcc_jit_type_is_integral(uint64)); > > > +  gcc_jit_type *int8 = > > > +    gcc_jit_context_get_int_type(ctxt, 1, 1); > > > +  CHECK (gcc_jit_type_is_integral(int8)); > > > +  gcc_jit_type *uint8 = > > > +    gcc_jit_context_get_int_type(ctxt, 1, 0); > > > +  CHECK (gcc_jit_type_is_integral(uint8)); > > > + > > > +  CHECK (!gcc_jit_type_is_vector(double_type)); > > > +  gcc_jit_type *vec_type = gcc_jit_type_get_vector (double_type, > > > 4); > > > +  gcc_jit_vector_type *vector_type = > > > gcc_jit_type_is_vector(vec_type); > > > +  CHECK (vector_type); > > > +  CHECK (vec_type != double_type); > > > +  CHECK_VALUE (gcc_jit_vector_type_get_element_type(vector_type), > > > double_type); > > > +  CHECK_VALUE (gcc_jit_vector_type_get_num_units(vector_type), 4); > > > + > > > +  CHECK (!gcc_jit_type_is_pointer(double_type)); > > > +  CHECK_VALUE > > > (gcc_jit_type_is_pointer(gcc_jit_type_get_pointer(double_type)), > > > double_type); > > > + > > > +  gcc_jit_type* params[2] = {int8, uint64}; > > > +  gcc_jit_type *function_ptr_type = > > > gcc_jit_context_new_function_ptr_type(ctxt, NULL, int64, 2, params, > > > 0); > > > +  gcc_jit_function_type *function_type = > > > gcc_jit_type_is_function_ptr_type (function_ptr_type); > > > +  CHECK (function_type); > > > +  int param_count = > > > gcc_jit_function_type_get_param_count(function_type); > > > +  CHECK_VALUE (param_count, 2); > > > +  gcc_jit_type *return_type = > > > gcc_jit_function_type_get_return_type(function_type); > > > +  CHECK_VALUE (return_type, int64); > > > +  gcc_jit_type *param1 = > > > gcc_jit_function_type_get_param_type(function_type, 0); > > > +  CHECK_VALUE (param1, int8); > > > +  gcc_jit_type *param2 = > > > gcc_jit_function_type_get_param_type(function_type, 1); > > > +  CHECK_VALUE (param2, uint64); > > > + > > > +  gcc_jit_field *field1 = gcc_jit_context_new_field (ctxt, NULL, > > > uint64, "field1"); > > > +  gcc_jit_field *field2 = gcc_jit_context_new_field (ctxt, NULL, > > > double_type, "field2"); > > > +  gcc_jit_field *fields[2] = { field1, field2 }; > > > +  gcc_jit_struct *struct_type = gcc_jit_context_new_struct_type > > > (ctxt, NULL, "testStruct", 2, fields); > > > +  CHECK_VALUE (gcc_jit_struct_get_field_count(struct_type), 2); > > > +  CHECK_VALUE (gcc_jit_struct_get_field(struct_type, 0), field1); > > > +  CHECK_VALUE (gcc_jit_struct_get_field(struct_type, 1), field2); > > > +  gcc_jit_struct *struct_ty = > > > gcc_jit_type_is_struct(gcc_jit_struct_as_type(struct_type)); > > > +  CHECK_VALUE (struct_ty, struct_type); > > > > > > I think this is missing test coverage for the various > > gcc_jit_type_is_* > > entrypoints gracefully returning NULL when the type passed to them is > > wrong (e.g. a non-struct type passed to gcc_jit_type_is_struct, etc) > > > > Hope this is constructive; sorry again about the delay in reviewing > > it. > > Dave > > > > > >