Bare bones of type verifier

Hi,
this patch adds bare bones of type checker.  You can call verify_type on any
type in the IL and see compiler bomb if some of invariants are broken.  So far
it only verify tests I already tested in last stage1 with my reverted variant
streaming patch https://gcc.gnu.org/ml/gcc-patches/2014-06/msg02454.html

The checks found interesting problems that was fixed. I have other tests and
fixes but would like to go incrementally.  Some of tests already broke again
with recent C++ align attribute changes (I think), see FIXME comments.
I plan to proceed with small steps becuase all the checks seems to trigger
fun issues.

The patch still fix on bug in ipa-chkp.c that is obvious enough - it is
cut&pasted from old code in cgraphunit.c that was updated same way.

I hope with early debug we are on the track of getting simplified gimple types,
but to make this without hitting too many surprises I think we first want to
document and verify our FE type representation and also verify what we need
in middle-end and drop the rest (in free lang data).

Placement of verify_type calls may seem bit random. Basic idea is to verify that
bad types do not hit LTO and are not produced by LTO type merging. In non-LTO path
we verify types at dwarf2out that is major consumer of the type variants.
I would be happy to place more calls/relocate existing to better places.

LTO-Boostrapped/regtested x86_64-linux without Ada. I am doing full run on
PPC64, but it fails on unrelated libgmp miscomplation I proably need to track down
first.

OK if testing passes?

Honza

	* tree.c: Include print-tree.h
	(verify_type_variant): New function.
	(verify_type): New function.
	* tree.h (verify_type): Declare.
	* tree-streamer-out.c (write_ts_type_common_tree_pointers): Verify type.
	* dwarf2out.c (dwarf2out_decl): Verify type.
	* ipa-chkp.c (chkp_copy_function_type_adding_bounds): Do not consider
	updated type to be variant.

	* lto.c (lto_fixup_state): Verify types.

On Tue, 28 Apr 2015, Jan Hubicka wrote:

> Hi,
> this patch adds bare bones of type checker.  You can call verify_type on any
> type in the IL and see compiler bomb if some of invariants are broken.  So far
> it only verify tests I already tested in last stage1 with my reverted variant
> streaming patch https://gcc.gnu.org/ml/gcc-patches/2014-06/msg02454.html
> 
> The checks found interesting problems that was fixed. I have other tests and
> fixes but would like to go incrementally.  Some of tests already broke again
> with recent C++ align attribute changes (I think), see FIXME comments.
> I plan to proceed with small steps becuase all the checks seems to trigger
> fun issues.
> 
> The patch still fix on bug in ipa-chkp.c that is obvious enough - it is
> cut&pasted from old code in cgraphunit.c that was updated same way.
> 
> I hope with early debug we are on the track of getting simplified gimple types,
> but to make this without hitting too many surprises I think we first want to
> document and verify our FE type representation and also verify what we need
> in middle-end and drop the rest (in free lang data).
> 
> Placement of verify_type calls may seem bit random. Basic idea is to verify that
> bad types do not hit LTO and are not produced by LTO type merging. In non-LTO path
> we verify types at dwarf2out that is major consumer of the type variants.
> I would be happy to place more calls/relocate existing to better places.

(early) dwarf2out is a good place, likewise verifying right after 
free-lang-data.  I agree that LTO type merging is also a good place.

I hope we get early dwarf2out merged soon and can enable free-lang-data
also for non-LTO compilation.

> LTO-Boostrapped/regtested x86_64-linux without Ada. I am doing full run on
> PPC64, but it fails on unrelated libgmp miscomplation I proably need to track down
> first.
> 
> OK if testing passes?

Please make sure to test _all_ languages (all,ada,obj-c++,go) and also
include multilibs in testing (thus -m64/-m32).

You don't verify that TYPE_CANONICAL is consistent nor that TYPE_CANONICAL
doesn't form a tree (TYPE_CANONICAL (t) == TYPE_CANONICAL (TYPE_CANONICAL 
(t))).

More comments below


> Honza
> 
> 	* tree.c: Include print-tree.h
> 	(verify_type_variant): New function.
> 	(verify_type): New function.
> 	* tree.h (verify_type): Declare.
> 	* tree-streamer-out.c (write_ts_type_common_tree_pointers): Verify type.
> 	* dwarf2out.c (dwarf2out_decl): Verify type.
> 	* ipa-chkp.c (chkp_copy_function_type_adding_bounds): Do not consider
> 	updated type to be variant.
> 
> 	* lto.c (lto_fixup_state): Verify types.
> Index: tree.c
> ===================================================================
> --- tree.c	(revision 222391)
> +++ tree.c	(working copy)
> @@ -102,6 +102,7 @@ along with GCC; see the file COPYING3.
>  #include "debug.h"
>  #include "intl.h"
>  #include "builtins.h"
> +#include "print-tree.h"
>  
>  /* Tree code classes.  */
>  
> @@ -12425,4 +12437,124 @@ element_mode (const_tree t)
>    return TYPE_MODE (t);
>  }
>  
> +/* Veirfy that basic properties of T match TV and thus T can be a variant of
> +   TV.  TV should be the more specified variant (i.e. the main variant).  */
> +
> +static bool
> +verify_type_variant (const_tree t, tree tv)
> +{
> +  if (TREE_CODE (t) != TREE_CODE (tv))
> +    {
> +      error ("type variant has different TREE_CODE");
> +      debug_tree (tv);
> +      return false;
> +    }
> +  if (COMPLETE_TYPE_P (t) && TYPE_SIZE (t) != TYPE_SIZE (tv))
> +    {
> +      error ("type variant has different TYPE_SIZE");
> +      debug_tree (tv);
> +      return false;
> +    }
> +  if (COMPLETE_TYPE_P (t) && TYPE_SIZE_UNIT (t) != TYPE_SIZE_UNIT (tv))
> +    {
> +      error ("type variant has different TYPE_SIZE_UNIT");
> +      debug_tree (tv);
> +      return false;
> +    }
> +  if (RECORD_OR_UNION_TYPE_P (t) && TYPE_VFIELD (t) != TYPE_VFIELD (tv))
> +    {
> +      error ("type variant has different TYPE_VFIELD");
> +      debug_tree (tv);
> +      return false;
> +    }
> +  if (((TREE_CODE (t) == ENUMERAL_TYPE && COMPLETE_TYPE_P (t))
> +	|| TREE_CODE (t) == INTEGER_TYPE
> +	|| TREE_CODE (t) == BOOLEAN_TYPE
> +	|| TREE_CODE (t) == REAL_TYPE
> +	|| TREE_CODE (t) == FIXED_POINT_TYPE)
> +       && (TYPE_MAX_VALUE (t) != TYPE_MAX_VALUE (tv)
> +	   || TYPE_MAX_VALUE (t) != TYPE_MAX_VALUE (tv)))
> +    {
> +      error ("type variant has different TYPE_MIN_VALUE");
> +      debug_tree (tv);
> +      return false;
> +    }
> +  if (TREE_CODE (t) == METHOD_TYPE
> +      && TYPE_METHOD_BASETYPE (t) != TYPE_METHOD_BASETYPE (tv))
> +    {
> +      error ("type variant has different TYPE_METHOD_BASETYPE");
> +      debug_tree (tv);
> +      return false;
> +    }
> +  /* FIXME: Be lax and allow TYPE_METHODS to be NULL.  This is a bug
> +     but affecting only the debug output.  */
> +  if (RECORD_OR_UNION_TYPE_P (t) && COMPLETE_TYPE_P (t)
> +      && TYPE_METHODS (t) && TYPE_METHODS (tv)
> +      && TYPE_METHODS (t) != TYPE_METHODS (tv))
> +    {
> +      error ("type variant has different TYPE_METHODS");
> +      debug_tree (tv);
> +      return false;
> +    }
> +  if (TREE_CODE (t) == OFFSET_TYPE
> +      && TYPE_OFFSET_BASETYPE (t) != TYPE_OFFSET_BASETYPE (tv))
> +    {
> +      error ("type variant has different TYPE_OFFSET_BASETYPE");
> +      debug_tree (tv);
> +      return false;
> +    }
> +  if (TREE_CODE (t) == ARRAY_TYPE
> +      && TYPE_ARRAY_MAX_SIZE (t) != TYPE_ARRAY_MAX_SIZE (tv))
> +    {
> +      error ("type variant has different TYPE_ARRAY_MAX_SIZE");
> +      debug_tree (tv);
> +      return false;
> +    }
> +  /* FIXME: Be lax and allow TYPE_BINFO to be missing in variant types
> +     or even type's main variant.  This is needed to make bootstrap pass
> +     and the bug seems new in GCC 5.
> +     C++ FE should be updated to make this consistent and we should check
> +     that TYPE_BINFO is always NULL for !COMPLETE_TYPE_P and otherwise there
> +     is a match with main variant.  */
> +  if (RECORD_OR_UNION_TYPE_P (t) && TYPE_BINFO (t) && TYPE_BINFO (tv)
> +      && TYPE_BINFO (t) != TYPE_BINFO (tv))
> +    {
> +      error ("type variant has different TYPE_BINFO");
> +      debug_tree (tv);
> +      error ("type variant's TYPE_BINFO");
> +      debug_tree (TYPE_BINFO (tv));
> +      error ("type's TYPE_BINFO");
> +      debug_tree (TYPE_BINFO (tv));
> +      return false;
> +    }
> +  return true;
> +}
> +
> +/* Verify type T.  */
> +
> +void
> +verify_type (const_tree t)
> +{
> +  bool error_found = false;
> +  tree mv = TYPE_MAIN_VARIANT (t);
> +  if (!mv)
> +    {
> +      error ("Main variant is not defined");
> +      error_found = true;
> +    }
> +  else if (mv != TYPE_MAIN_VARIANT (mv))
> +    {
> +      error ("TYPE_MAIN_VARAINT has different TYPE_MAIN_VARIANT");
> +      debug_tree (mv);
> +      error_found = true;
> +    }
> +  else if (t != mv && !verify_type_variant (t, mv))
> +    error_found = true;
> +  if (error_found)
> +    {
> +      debug_tree (const_cast <tree> (t));
> +      internal_error ("verify_type failed");
> +    }
> +}
> +
>  #include "gt-tree.h"
> Index: tree.h
> ===================================================================
> --- tree.h	(revision 222391)
> +++ tree.h	(working copy)
> @@ -4495,6 +4495,7 @@ extern tree drop_tree_overflow (tree);
>  extern int tree_map_base_eq (const void *, const void *);
>  extern unsigned int tree_map_base_hash (const void *);
>  extern int tree_map_base_marked_p (const void *);
> +extern void DEBUG_FUNCTION verify_type (const_tree t);
>  
>  #define tree_map_eq tree_map_base_eq
>  extern unsigned int tree_map_hash (const void *);
> Index: lto/lto.c
> ===================================================================
> --- lto/lto.c	(revision 222391)
> +++ lto/lto.c	(working copy)
> @@ -2844,6 +2844,10 @@ lto_fixup_state (struct lto_in_decl_stat
>        for (i = 0; i < vec_safe_length (trees); i++)
>  	{
>  	  tree t = (*trees)[i];
> +#ifdef ENABLE_CHECKING
> +	  if (TYPE_P (t))
> +	    verify_type (t);
> +#endif
>  	  if (VAR_OR_FUNCTION_DECL_P (t)
>  	      && (TREE_PUBLIC (t) || DECL_EXTERNAL (t)))
>  	    (*trees)[i] = lto_symtab_prevailing_decl (t);
> Index: tree-streamer-out.c
> ===================================================================
> --- tree-streamer-out.c	(revision 222391)
> +++ tree-streamer-out.c	(working copy)
> @@ -721,6 +721,9 @@ static void
>  write_ts_type_common_tree_pointers (struct output_block *ob, tree expr,
>  				    bool ref_p)
>  {
> +#ifdef ENABLE_CHECKING
> +  verify_type (expr);
> +#endif

As said I think that doing this after free-lang-data is better.  Like
here:

  /* Traverse every type found freeing its language data.  */
  FOR_EACH_VEC_ELT (fld.types, i, t)
    free_lang_data_in_type (t);

do another loop verifying types.

>    stream_write_tree (ob, TYPE_SIZE (expr), ref_p);
>    stream_write_tree (ob, TYPE_SIZE_UNIT (expr), ref_p);
>    stream_write_tree (ob, TYPE_ATTRIBUTES (expr), ref_p);
> Index: dwarf2out.c
> ===================================================================
> --- dwarf2out.c	(revision 222391)
> +++ dwarf2out.c	(working copy)
> @@ -21264,6 +21264,11 @@ dwarf2out_decl (tree decl)
>  {
>    dw_die_ref context_die = comp_unit_die ();
>  
> +#ifdef ENABLE_CHECKING
> +  if (TREE_TYPE (decl))
> +     verify_type (TREE_TYPE (decl));
> +#endif
> +

Hmm, this has the chance to verify types multiple times, no?  Wouldn't
gen_type_die_with_usage be a better place to verify the type we create
the DIE for?

So besides placing and doing more checking the patch looks ok to me.

Thanks,
Richard.

>    switch (TREE_CODE (decl))
>      {
>      case ERROR_MARK:
> Index: ipa-chkp.c
> ===================================================================
> --- ipa-chkp.c	(revision 222391)
> +++ ipa-chkp.c	(working copy)
> @@ -244,7 +244,7 @@ tree
>  chkp_copy_function_type_adding_bounds (tree orig_type)
>  {
>    tree type;
> -  tree arg_type, attrs, t;
> +  tree arg_type, attrs;
>    unsigned len = list_length (TYPE_ARG_TYPES (orig_type));
>    unsigned *indexes = XALLOCAVEC (unsigned, len);
>    unsigned idx = 0, new_idx = 0;
> @@ -327,20 +327,6 @@ chkp_copy_function_type_adding_bounds (t
>        TYPE_ATTRIBUTES (type) = attrs;
>      }
>  
> -  t = TYPE_MAIN_VARIANT (orig_type);
> -  if (orig_type != t)
> -    {
> -      TYPE_MAIN_VARIANT (type) = t;
> -      TYPE_NEXT_VARIANT (type) = TYPE_NEXT_VARIANT (t);
> -      TYPE_NEXT_VARIANT (t) = type;
> -    }
> -  else
> -    {
> -      TYPE_MAIN_VARIANT (type) = type;
> -      TYPE_NEXT_VARIANT (type) = NULL;
> -    }
> -
> -
>    return type;
>  }

Hi,
I will fix the free count issue - my internet access in China was bit sporadic
and I hoped you will know why the count seem to disagree between PPC64 and Linux
builds. In meantime PPC64 stopped to build for me.
> 
> (early) dwarf2out is a good place, likewise verifying right after 
> free-lang-data.  I agree that LTO type merging is also a good place.
> 
> I hope we get early dwarf2out merged soon and can enable free-lang-data
> also for non-LTO compilation.
> 
> > LTO-Boostrapped/regtested x86_64-linux without Ada. I am doing full run on
> > PPC64, but it fails on unrelated libgmp miscomplation I proably need to track down
> > first.
> > 
> > OK if testing passes?
> 
> Please make sure to test _all_ languages (all,ada,obj-c++,go) and also
> include multilibs in testing (thus -m64/-m32).
> 
> You don't verify that TYPE_CANONICAL is consistent nor that TYPE_CANONICAL
> doesn't form a tree (TYPE_CANONICAL (t) == TYPE_CANONICAL (TYPE_CANONICAL 
> (t))).

Yep, I have this one in queue, but of course it fires, thus it is not in the initial patch.
> > +#ifdef ENABLE_CHECKING
> > +  verify_type (expr);
> > +#endif
> 
> As said I think that doing this after free-lang-data is better.  Like
> here:
> 
>   /* Traverse every type found freeing its language data.  */
>   FOR_EACH_VEC_ELT (fld.types, i, t)
>     free_lang_data_in_type (t);
> 
> do another loop verifying types.

OK, we however build new types in middle end (ipa-icf/ipa-split/ipa-sra),
so perhaps we can check on both places?
> 
> >    stream_write_tree (ob, TYPE_SIZE (expr), ref_p);
> >    stream_write_tree (ob, TYPE_SIZE_UNIT (expr), ref_p);
> >    stream_write_tree (ob, TYPE_ATTRIBUTES (expr), ref_p);
> > Index: dwarf2out.c
> > ===================================================================
> > --- dwarf2out.c	(revision 222391)
> > +++ dwarf2out.c	(working copy)
> > @@ -21264,6 +21264,11 @@ dwarf2out_decl (tree decl)
> >  {
> >    dw_die_ref context_die = comp_unit_die ();
> >  
> > +#ifdef ENABLE_CHECKING
> > +  if (TREE_TYPE (decl))
> > +     verify_type (TREE_TYPE (decl));
> > +#endif
> > +
> 
> Hmm, this has the chance to verify types multiple times, no?  Wouldn't
> gen_type_die_with_usage be a better place to verify the type we create
> the DIE for?

That looks better indeed.  The checks are not paritcularly expensive though.

Honza

On April 28, 2015 11:05:44 AM GMT+02:00, Jan Hubicka <hubicka@ucw.cz> wrote:
>Hi,
>I will fix the free count issue - my internet access in China was bit
>sporadic
>and I hoped you will know why the count seem to disagree between PPC64
>and Linux
>builds. In meantime PPC64 stopped to build for me.
>> 
>> (early) dwarf2out is a good place, likewise verifying right after 
>> free-lang-data.  I agree that LTO type merging is also a good place.
>> 
>> I hope we get early dwarf2out merged soon and can enable
>free-lang-data
>> also for non-LTO compilation.
>> 
>> > LTO-Boostrapped/regtested x86_64-linux without Ada. I am doing full
>run on
>> > PPC64, but it fails on unrelated libgmp miscomplation I proably
>need to track down
>> > first.
>> > 
>> > OK if testing passes?
>> 
>> Please make sure to test _all_ languages (all,ada,obj-c++,go) and
>also
>> include multilibs in testing (thus -m64/-m32).
>> 
>> You don't verify that TYPE_CANONICAL is consistent nor that
>TYPE_CANONICAL
>> doesn't form a tree (TYPE_CANONICAL (t) == TYPE_CANONICAL
>(TYPE_CANONICAL 
>> (t))).
>
>Yep, I have this one in queue, but of course it fires, thus it is not
>in the initial patch.
>> > +#ifdef ENABLE_CHECKING
>> > +  verify_type (expr);
>> > +#endif
>> 
>> As said I think that doing this after free-lang-data is better.  Like
>> here:
>> 
>>   /* Traverse every type found freeing its language data.  */
>>   FOR_EACH_VEC_ELT (fld.types, i, t)
>>     free_lang_data_in_type (t);
>> 
>> do another loop verifying types.
>
>OK, we however build new types in middle end
>(ipa-icf/ipa-split/ipa-sra),
>so perhaps we can check on both places?
>> 
>> >    stream_write_tree (ob, TYPE_SIZE (expr), ref_p);
>> >    stream_write_tree (ob, TYPE_SIZE_UNIT (expr), ref_p);
>> >    stream_write_tree (ob, TYPE_ATTRIBUTES (expr), ref_p);
>> > Index: dwarf2out.c
>> > ===================================================================
>> > --- dwarf2out.c	(revision 222391)
>> > +++ dwarf2out.c	(working copy)
>> > @@ -21264,6 +21264,11 @@ dwarf2out_decl (tree decl)
>> >  {
>> >    dw_die_ref context_die = comp_unit_die ();
>> >  
>> > +#ifdef ENABLE_CHECKING
>> > +  if (TREE_TYPE (decl))
>> > +     verify_type (TREE_TYPE (decl));
>> > +#endif
>> > +
>> 
>> Hmm, this has the chance to verify types multiple times, no? 
>Wouldn't
>> gen_type_die_with_usage be a better place to verify the type we
>create
>> the DIE for?
>
>That looks better indeed.  The checks are not paritcularly expensive
>though.

+ if (((TREE_CODE (t) == ENUMERAL_TYPE && COMPLETE_TYPE_P (t))
+	|| TREE_CODE (t) == INTEGER_TYPE
+	|| TREE_CODE (t) == BOOLEAN_TYPE
+	|| TREE_CODE (t) == REAL_TYPE
+	|| TREE_CODE (t) == FIXED_POINT_TYPE)
+ && (TYPE_MAX_VALUE (t) != TYPE_MAX_VALUE (tv)
+	 || TYPE_MAX_VALUE (t) != TYPE_MAX_VALUE (tv)))
+ {
+ error ("type variant has different TYPE_MIN_VALUE");
+ debug_tree (tv);
+ return false;
+ }

The second || check of TYPE_MAX_VALUE should probably be something else, maybe TYPE_MIN_VALUE ?

Why don't we warn about such useless || where both hands are identical? :)

Thanks,
>
>Honza

On April 28, 2015 6:01:24 PM GMT+02:00, Bernhard Reutner-Fischer <rep.dot.nop@gmail.com> wrote:
>On April 28, 2015 11:05:44 AM GMT+02:00, Jan Hubicka <hubicka@ucw.cz>
>wrote:
>>Hi,
>>I will fix the free count issue - my internet access in China was bit
>>sporadic
>>and I hoped you will know why the count seem to disagree between PPC64
>>and Linux
>>builds. In meantime PPC64 stopped to build for me.
>>> 
>>> (early) dwarf2out is a good place, likewise verifying right after 
>>> free-lang-data.  I agree that LTO type merging is also a good place.
>>> 
>>> I hope we get early dwarf2out merged soon and can enable
>>free-lang-data
>>> also for non-LTO compilation.
>>> 
>>> > LTO-Boostrapped/regtested x86_64-linux without Ada. I am doing
>full
>>run on
>>> > PPC64, but it fails on unrelated libgmp miscomplation I proably
>>need to track down
>>> > first.
>>> > 
>>> > OK if testing passes?
>>> 
>>> Please make sure to test _all_ languages (all,ada,obj-c++,go) and
>>also
>>> include multilibs in testing (thus -m64/-m32).
>>> 
>>> You don't verify that TYPE_CANONICAL is consistent nor that
>>TYPE_CANONICAL
>>> doesn't form a tree (TYPE_CANONICAL (t) == TYPE_CANONICAL
>>(TYPE_CANONICAL 
>>> (t))).
>>
>>Yep, I have this one in queue, but of course it fires, thus it is not
>>in the initial patch.
>>> > +#ifdef ENABLE_CHECKING
>>> > +  verify_type (expr);
>>> > +#endif
>>> 
>>> As said I think that doing this after free-lang-data is better. 
>Like
>>> here:
>>> 
>>>   /* Traverse every type found freeing its language data.  */
>>>   FOR_EACH_VEC_ELT (fld.types, i, t)
>>>     free_lang_data_in_type (t);
>>> 
>>> do another loop verifying types.
>>
>>OK, we however build new types in middle end
>>(ipa-icf/ipa-split/ipa-sra),
>>so perhaps we can check on both places?
>>> 
>>> >    stream_write_tree (ob, TYPE_SIZE (expr), ref_p);
>>> >    stream_write_tree (ob, TYPE_SIZE_UNIT (expr), ref_p);
>>> >    stream_write_tree (ob, TYPE_ATTRIBUTES (expr), ref_p);
>>> > Index: dwarf2out.c
>>> >
>===================================================================
>>> > --- dwarf2out.c	(revision 222391)
>>> > +++ dwarf2out.c	(working copy)
>>> > @@ -21264,6 +21264,11 @@ dwarf2out_decl (tree decl)
>>> >  {
>>> >    dw_die_ref context_die = comp_unit_die ();
>>> >  
>>> > +#ifdef ENABLE_CHECKING
>>> > +  if (TREE_TYPE (decl))
>>> > +     verify_type (TREE_TYPE (decl));
>>> > +#endif
>>> > +
>>> 
>>> Hmm, this has the chance to verify types multiple times, no? 
>>Wouldn't
>>> gen_type_die_with_usage be a better place to verify the type we
>>create
>>> the DIE for?
>>
>>That looks better indeed.  The checks are not paritcularly expensive
>>though.
>
>+ if (((TREE_CODE (t) == ENUMERAL_TYPE && COMPLETE_TYPE_P (t))
>+	|| TREE_CODE (t) == INTEGER_TYPE
>+	|| TREE_CODE (t) == BOOLEAN_TYPE
>+	|| TREE_CODE (t) == REAL_TYPE
>+	|| TREE_CODE (t) == FIXED_POINT_TYPE)
>+ && (TYPE_MAX_VALUE (t) != TYPE_MAX_VALUE (tv)
>+	 || TYPE_MAX_VALUE (t) != TYPE_MAX_VALUE (tv)))
>+ {
>+ error ("type variant has different TYPE_MIN_VALUE");
>+ debug_tree (tv);
>+ return false;
>+ }
>
>The second || check of TYPE_MAX_VALUE should probably be something
>else, maybe TYPE_MIN_VALUE ?
>
>Why don't we warn about such useless || where both hands are identical?
>:)

Also: s/TYPE_MAIN_VARAINT/TYPE_MAIN_VARIANT/g
Thanks,

> >though.
> 
> + if (((TREE_CODE (t) == ENUMERAL_TYPE && COMPLETE_TYPE_P (t))
> +	|| TREE_CODE (t) == INTEGER_TYPE
> +	|| TREE_CODE (t) == BOOLEAN_TYPE
> +	|| TREE_CODE (t) == REAL_TYPE
> +	|| TREE_CODE (t) == FIXED_POINT_TYPE)
> + && (TYPE_MAX_VALUE (t) != TYPE_MAX_VALUE (tv)
> +	 || TYPE_MAX_VALUE (t) != TYPE_MAX_VALUE (tv)))
> + {
> + error ("type variant has different TYPE_MIN_VALUE");
> + debug_tree (tv);
> + return false;
> + }
> 
> The second || check of TYPE_MAX_VALUE should probably be something else, maybe TYPE_MIN_VALUE ?
> 
> Why don't we warn about such useless || where both hands are identical? :)

Thanks for both corrections! Something I crept in at last moment when restructuring the code
(I want to re-use the variant checking for ODR varaints and also in limited form for canonical
types - combined with existing code to verify canonical type sanity in C++ FE)

Yep, warning would be cute, though it may be bit hard to interpret when the tests are produced
by different maco expansions/inlines.

Honza
> 
> Thanks,
> >
> >Honza
>