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

[Andrew Haley <aph at redhat dot com>]

Richard Guenther wrote:
> 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.

I think so.  I'm not sure I understand how all this happened.

There was a bug involving bitfield references in C++, and fixing
this bug somehow caused jboolean no longer to be treated as a
1-bit boolean type.  Aoliva fixed the "+=" in libjava, and this was
enough to fix the regressions.

Then you tried to make __java_boolean a true bool, in order to
preserve its special semantics on increment.  But this broke Darwin,
where C++ bool is 4 bytes wide.  So, was __java_boolean a true bool
before the change that fixed the bug involving bitfield references
in C++ ?

So, if you revert this patch, there will be no ABI change, but there
will be a minor change in the semantics of __java_boolean, which will
become an integer type.

Andrew.