Re: [PATCH][C++] Make __java_boolean a true bool (PR 33887)

[Richard Guenther <rguenther at suse dot de>]

On Tue, 29 Jan 2008, Andrew Haley wrote:

> Richard Guenther wrote:
> > On Tue, 29 Jan 2008, Richard Guenther wrote:
> > 
> > > On Tue, 29 Jan 2008, Richard Guenther wrote:
> > >
> > > > 2008-01-29  Richard Guenther  <rguenther@suse.de>
> > > >
> > > >  Revert
> > > >  PR c++/33887
> > > >  * decl.c (record_builtin_java_type): Make __java_boolean
> > > >  a variant of bool.
> > > >  * typeck.c (structural_comptypes): Move TYPE_FOR_JAVA check
> > > >  after TYPE_MAIN_VARIANT check.
> > > >
> > > >  * typeck.c (build_unary_op): Reject -- on all boolean types.
> > > >  * decl.c (record_builtin_java_type): Make __java_boolean
> > > >  a BOOLEAN_TYPE.
> > > >  * cvt.c (type_promotes_to): All BOOLEAN_TYPEs promote to int.
> > > >  * mangle.c (write_template_arg_literal): All BOOLEAN_TYPE
> > > >  constants should be either one or zero.
> > > >
> > 
> > And testing now reveals why I chose the type-variant approach.  For
> > 
> > void foo(void)
> > {
> >   __builtin_va_list va;
> >   (__java_boolean)__builtin_va_arg (va, int);
> > }
> > 
> > (reduced from jni.ii)
> > 
> > We do build_c_cast which performs the conversion via build_static_cast_1
> > and returns VA_ARG_EXPR <va> != 0 of type boolean_type_node(!), which
> > we then try to convert to __java_boolean node again here:
> > 
> >       /* If the type of RESULT does not match TYPE, perform a
> >          const_cast to make it match.  If the static_cast or
> >          reinterpret_cast succeeded, we will differ by at most
> >          cv-qualification, so the follow-on const_cast is guaranteed
> >          to succeed.  */
> >       if (!same_type_p (non_reference (type), non_reference (result_type)))
> >         {
> >           result = build_const_cast_1 (type, result, false, &valid_p);
> >           gcc_assert (valid_p);
> >         }
> > 
> > and ICE.
> > 
> > This is where I gave up previosly (imagining there can be similar
> > problems elsewhere).  It seems to be a twisted maze to fix this
> > up for real (using the "correct" boolean type here), but we of
> > course can fix it up via
> > 
> > @@ -5591,7 +5590,9 @@ build_c_cast (tree type, tree expr)
> >          reinterpret_cast succeeded, we will differ by at most
> >          cv-qualification, so the follow-on const_cast is guaranteed
> >          to succeed.  */
> > -      if (!same_type_p (non_reference (type), non_reference (result_type)))
> > +      if (!(TREE_CODE (non_reference (type)) == BOOLEAN_TYPE
> > +           && TREE_CODE (non_reference (result_type)) == BOOLEAN_TYPE)
> > +         && !same_type_p (non_reference (type), non_reference
> > (result_type)))
> >         {
> >           result = build_const_cast_1 (type, result, false, &valid_p);
> >           gcc_assert (valid_p);
> > 
> > but then end up not actually converting to __java_boolean type.
> > 
> > Now, appearantly jboolean and bool are _not_ the same on darwin, so
> > the above would not work there.
> > 
> > Ideas?
> 
> Just make jboolean a byte type on Darwin.  The only place it might matter
> is when someone (say) increments a jboolean and expects it to have semantics
> different from a byte, and that isn't massively important.  We never said
> jboolean would have the exact same semantics as bool anyway.

Well, then we can just revert this patch entirely.  But I'm curious if
things that used to work before (like the single libjava occurance we
fixed) start to fail now.

But, probably better than breaking the ABI on darwin anyway.

So, if nobody objects I'm going to revert this patch tomorrow.

Thanks,
Richard.