Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(88)

Issue 12382043: code review 12382043: spec: && and || permit operands of different boolean types

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 years, 8 months ago by gri
Modified:
10 years ago
Reviewers:
r, ken2, rsc, iant, adonovan
CC:
golang-codereviews
Visibility:
Public.

Description

spec: && and || permit operands of different boolean types If the operand types differ (newly permitted), the result is an untyped boolean value. Otherwise the result type is the same as the (typed) operand's type. This is a backward-compatible language change. The actual change is in the section for logical operators. However, it required adjustments to the section on general operators. The text in that section would now more naturally fit into the sections for the different operators. Leaving that cleanup for a separate CL to keep this one simple. Fixes issue 4336.

Patch Set 1 #

Patch Set 2 : diff -r 4a3d328fe8e9 https://code.google.com/p/go #

Patch Set 3 : diff -r 4a3d328fe8e9 https://code.google.com/p/go #

Total comments: 2

Patch Set 4 : diff -r 4a3d328fe8e9 https://code.google.com/p/go #

Unified diffs Side-by-side diffs Delta from patch set Stats (+23 lines, -11 lines) Patch
M doc/go_spec.html View 1 2 3 3 chunks +23 lines, -11 lines 0 comments Download

Messages

Total messages: 17
gri
Hello r@golang.org, rsc@golang.org, iant@golang.org, ken@golang.org (cc: golang-dev@googlegroups.com), I'd like you to review this change to ...
10 years, 8 months ago (2013-08-02 22:30:41 UTC) #1
r
this is easy but it seems odd to have a special case like this. what's ...
10 years, 8 months ago (2013-08-02 22:39:08 UTC) #2
r
OK. Here is the piece of reasoning, from the issue: I think we should fix ...
10 years, 8 months ago (2013-08-02 22:42:45 UTC) #3
gri
Also, since the result may depend on the value of the first operand only, arguably ...
10 years, 8 months ago (2013-08-02 22:51:36 UTC) #4
r
LGTM but wait for others
10 years, 8 months ago (2013-08-02 22:56:37 UTC) #5
iant
https://codereview.appspot.com/12382043/diff/6001/doc/go_spec.html File doc/go_spec.html (right): https://codereview.appspot.com/12382043/diff/6001/doc/go_spec.html#newcode2922 doc/go_spec.html:2922: For <a href="#Arithmetic_operators">arithmetic operators</a>, the operand I wonder if ...
10 years, 8 months ago (2013-08-02 23:51:41 UTC) #6
gri
https://codereview.appspot.com/12382043/diff/6001/doc/go_spec.html File doc/go_spec.html (right): https://codereview.appspot.com/12382043/diff/6001/doc/go_spec.html#newcode2922 doc/go_spec.html:2922: For <a href="#Arithmetic_operators">arithmetic operators</a>, the operand On 2013/08/02 23:51:41, ...
10 years, 8 months ago (2013-08-02 23:58:45 UTC) #7
iant
LGTM
10 years, 8 months ago (2013-08-03 00:00:28 UTC) #8
rsc
LGTM
10 years, 8 months ago (2013-08-03 01:15:37 UTC) #9
adonovan
(Sorry I didn't see this earlier.) I'm not convinced this is a backwards-compatible change. Consider: ...
10 years, 8 months ago (2013-08-03 03:07:10 UTC) #10
rsc
I guess we should say that if the types of both sides agree, the result ...
10 years, 8 months ago (2013-08-03 03:17:54 UTC) #11
gri
Sigh... I will send out an updated version later this weekend. - gri On Fri, ...
10 years, 8 months ago (2013-08-03 04:37:30 UTC) #12
gri
Hello r@golang.org, rsc@golang.org, iant@golang.org, ken@golang.org, adonovan@google.com (cc: golang-dev@googlegroups.com), Please take another look.
10 years, 8 months ago (2013-08-03 05:53:35 UTC) #13
gri
PS: I am not so strongly convinced anymore that this change is better than what ...
10 years, 8 months ago (2013-08-03 06:03:27 UTC) #14
gobot
Replacing golang-dev with golang-codereviews.
10 years, 3 months ago (2013-12-20 16:25:56 UTC) #15
rsc
R=close assuming this is stale
10 years ago (2014-03-05 19:40:13 UTC) #16
gri
10 years ago (2014-03-05 19:44:20 UTC) #17
Yes, the corresponding issue 4336 would amount to a backward-incompatible
language change. Postponed indefinitely.
- gri


On Wed, Mar 5, 2014 at 11:40 AM, <rsc@golang.org> wrote:

> R=close
>
> assuming this is stale
>
>
> https://codereview.appspot.com/12382043/
>
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b