code review 6855061: cmd/gc: Make sure bools lose idealness when used with l...

cmd/gc: Make sure bools lose idealness when used with logical operators.

Bools from comparisons can be assigned to all bool types, but this idealness would propagate through logical operators when the result should have been lowered to a non-ideal form.

Fixes issue 3924.

[DMorsing, 2012-11-18 18:16:46]

Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://code.google.com/p/go/

[remyoudompheng, 2012-11-18 23:04:27]

I don't understand why the comparison result should be lowered to a concrete
type before being assigned to a variable.

[DMorsing, 2012-11-18 23:23:10]

On 2012/11/18 23:04:27, remyoudompheng wrote:
> I don't understand why the comparison result should be lowered to a concrete
> type before being assigned to a variable.

&& and || are logical operators. Their results are not supposed to be ideal.
Before this fix, they would become ideal if both operands were.

You could stretch the wording of the spec a bit and say that the idealness is
part of the resulting type. I think the wording about context means that they
should be lowered to a bool when used as both operands to a logical operator.

[r, 2012-11-18 23:35:23]

Here's the relevant sentence from the spec:

Logical operators apply to boolean values and yield a result of the
same type as the operands.

Given that sentence and the examples in the provided test program,
this CL looks right to me. I am not qualified to comment on the
compiler change however.

-rob

[remyoudompheng, 2012-11-19 01:19:12]

On 2012/11/19 Rob Pike <r@golang.org> wrote:
> Here's the relevant sentence from the spec:
>
> Logical operators apply to boolean values and yield a result of the
> same type as the operands.
>
> Given that sentence and the examples in the provided test program,
> this CL looks right to me. I am not qualified to comment on the
> compiler change however.

Then I am confused by the subtle distinction between:
* untyped non-constant boolean resulting from a comparison
* untyped constant boolean used in constant expressions.

In the first case, using a logical operator will coerce the operands
to their typed flavours. In the second case, the result of a logical
operation remains an untyped boolean.

It feels like that distinction introduces a notion of untyped
non-constant value, which doesn't exist elsewhere in the spec. Maybe I
just have a twisted mind and I should rather think that the result of
a comparison is "bool except when it is immediately assigned to a
variable of any boolean type, in which case the assignment is legal".

In the sentence "The result of a comparison can be assigned to any
boolean type. If the context does not demand a specific boolean type,
the result has type bool.", the word context is vague and can suggest
the code in the CL test case is valid.

Rémy.

[rsc, 2012-11-19 13:31:08]

I am quite confused about whether this is the behavior we want. The
distinction about which computed bools are 'ideal' and which are not
seems too unpredictable. There are other open issues about bools too.
I wonder if we should reexamine what the rules are.

Russ

[mtj1, 2012-11-19 14:01:39]

...and maybe add the missing boolean operations & and | friends?

On Mon, Nov 19, 2012 at 5:31 AM, Russ Cox <rsc@golang.org> wrote:

> I am quite confused about whether this is the behavior we want. The
> distinction about which computed bools are 'ideal' and which are not
> seems too unpredictable. There are other open issues about bools too.
> I wonder if we should reexamine what the rules are.
>
> Russ
>



-- 
Michael T. Jones | Chief Technology Advocate  | mtj@google.com |  +1
650-335-5765

[DMorsing, 2012-11-20 16:09:01]

On Mon, Nov 19, 2012 at 2:31 PM, Russ Cox <rsc@golang.org> wrote:
> I am quite confused about whether this is the behavior we want. The
> distinction about which computed bools are 'ideal' and which are not
> seems too unpredictable. There are other open issues about bools too.
> I wonder if we should reexamine what the rules are.
>
> Russ

Would there be any adverse effects by defining logical operations as
having the convertible bool result? I'm struggling to see where that
would be a problem.

[remyoudompheng, 2012-11-20 17:30:45]

On 2012/11/20 16:09:01, DMorsing wrote:
> On Mon, Nov 19, 2012 at 2:31 PM, Russ Cox <mailto:rsc@golang.org> wrote:
> > I am quite confused about whether this is the behavior we want. The
> > distinction about which computed bools are 'ideal' and which are not
> > seems too unpredictable. There are other open issues about bools too.
> > I wonder if we should reexamine what the rules are.
> >
> > Russ
> 
> Would there be any adverse effects by defining logical operations as
> having the convertible bool result? I'm struggling to see where that
> would be a problem.

Yes: if a and b have type MyBool, a && b should have type MyBool. I would tend
to have the following rules.

A boolean value may be typed or untyped.
* An untyped boolean constant is untyped.
* The result of a comparison (==, !=, <=, >=, <, >) is untyped.

A boolean value is assignable to type T:
* if T is an interface type. If the value is untyped, it is converted to bool.
* if T is a boolean type and the value is untyped.
* if T is a boolean type and the value has type T.

Declaration-assignment of a variable with an untyped boolean value has type
bool.

The result of a logical operation (&&, ||, !):
* has type T if one of its operands has type T. The operation is illegal if the
other operand is not assignable to T.
* is untyped when both operands are untyped

Something I don't like with these rules is that if a and b have boolean type T1,
and T2 is a boolean type distinct from T1, a == b and a != b are assignable to
T2. I would naively interpret a != b as "a XOR b" and it would be a typed value
of type T1. But I prefer to have consistency with numeric types and arithmetic
operators.

[DMorsing, 2012-11-20 17:58:06]

On 2012/11/20 17:30:45, remyoudompheng wrote:
> On 2012/11/20 16:09:01, DMorsing wrote:
> > On Mon, Nov 19, 2012 at 2:31 PM, Russ Cox <mailto:rsc@golang.org> wrote:
> > > I am quite confused about whether this is the behavior we want. The
> > > distinction about which computed bools are 'ideal' and which are not
> > > seems too unpredictable. There are other open issues about bools too.
> > > I wonder if we should reexamine what the rules are.
> > >
> > > Russ
> > 
> > Would there be any adverse effects by defining logical operations as
> > having the convertible bool result? I'm struggling to see where that
> > would be a problem.
> 
> Yes: if a and b have type MyBool, a && b should have type MyBool. I would tend
> to have the following rules.

This is the status quo for gc. All that's needed is a clarification in the
specification.

However, I don't think we need to add the concept of "untyped bool" to the spec.
I'd much rather have the current wording (results from comparisons can be
assigned to any bool type) and add a caveat in the section about logical
operators. Something to the tune of "If both operands are results from
comparisons, the result of the logical operator is assignable to any bool type".

We still need to figure out what the situation is with gccgo. I'm not sure how
consistent the 2 implementations are with how they handle booleans.

[rsc, 2012-11-26 19:01:04]

LGTM

I think the spec might be helped by some tweaks, but this matches the
spec so it should go in.

[DMorsing, 2012-11-26 21:16:04]

On Mon, Nov 26, 2012 at 8:01 PM, Russ Cox <rsc@golang.org> wrote:
> LGTM
>
> I think the spec might be helped by some tweaks, but this matches the
> spec so it should go in.

I'm committing this for now, but I think we need to reevaluate the
handling of bools. Right now the spec is vague, and the
implementations are inconsistent on how they handle them.

[rsc, 2012-11-26 21:16:40]

I thought this CL was making gc consistent with gccgo?
What's inconsistent still?

[DMorsing, 2012-11-26 21:21:08]

On Mon, Nov 26, 2012 at 10:16 PM, Russ Cox <rsc@golang.org> wrote:
> I thought this CL was making gc consistent with gccgo?
> What's inconsistent still?

Issue 3923 and 3915

[DMorsing, 2012-11-26 21:23:18]

*** Submitted as http://code.google.com/p/go/source/detail?r=81b14ef2226a ***

cmd/gc: Make sure bools lose idealness when used with logical operators.

Bools from comparisons can be assigned to all bool types, but this idealness
would propagate through logical operators when the result should have been
lowered to a non-ideal form.

Fixes issue 3924.

R=golang-dev, remyoudompheng, r, rsc, mtj
CC=golang-dev
http://codereview.appspot.com/6855061