|
|
Created:
11 years, 5 months ago by DMorsing Modified:
11 years, 4 months ago Reviewers:
CC:
golang-dev, remyoudompheng, r, rsc, mtj1 Visibility:
Public. |
Descriptioncmd/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.
Patch Set 1 #Patch Set 2 : diff -r bbc0024af4a4 https://code.google.com/p/go/ #Patch Set 3 : diff -r bbc0024af4a4 https://code.google.com/p/go/ #Patch Set 4 : diff -r 3fc28eec1b38 https://code.google.com/p/go/ #
MessagesTotal messages: 15
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/
Sign in to reply to this message.
I don't understand why the comparison result should be lowered to a concrete type before being assigned to a variable.
Sign in to reply to this message.
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.
Sign in to reply to this message.
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
Sign in to reply to this message.
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.
Sign in to reply to this message.
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
Sign in to reply to this message.
...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
Sign in to reply to this message.
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.
Sign in to reply to this message.
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.
Sign in to reply to this message.
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.
Sign in to reply to this message.
LGTM I think the spec might be helped by some tweaks, but this matches the spec so it should go in.
Sign in to reply to this message.
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.
Sign in to reply to this message.
I thought this CL was making gc consistent with gccgo? What's inconsistent still?
Sign in to reply to this message.
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
Sign in to reply to this message.
*** 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
Sign in to reply to this message.
|