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

Issue 13367051: code review 13367051: cmd/gc: allow append and complex builtins to accept 2-r...

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 years, 6 months ago by cmang
Modified:
11 years ago
Reviewers:
rsc
CC:
rsc, adonovan, dave_cheney.net, golang-codereviews
Visibility:
Public.

Description

cmd/gc: allow append and complex builtins to accept 2-result call expression as first argument. Fixes issue 5793.

Patch Set 1 #

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

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

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

Patch Set 5 : diff -r 63f29041c1cf https://code.google.com/p/go #

Total comments: 2

Patch Set 6 : diff -r 63f29041c1cf https://code.google.com/p/go #

Patch Set 7 : diff -r 63f29041c1cf https://code.google.com/p/go #

Patch Set 8 : diff -r 63f29041c1cf https://code.google.com/p/go #

Patch Set 9 : diff -r 63f29041c1cf https://code.google.com/p/go #

Total comments: 1

Patch Set 10 : diff -r acd164353815 https://code.google.com/p/go #

Patch Set 11 : diff -r acd164353815 https://code.google.com/p/go #

Patch Set 12 : diff -r acd164353815 https://code.google.com/p/go #

Unified diffs Side-by-side diffs Delta from patch set Stats (+86 lines, -12 lines) Patch
M src/cmd/gc/inl.c View 1 1 chunk +2 lines, -0 lines 0 comments Download
M src/cmd/gc/order.c View 1 2 3 4 5 6 7 1 chunk +2 lines, -0 lines 0 comments Download
M src/cmd/gc/typecheck.c View 1 2 3 4 5 6 7 8 9 2 chunks +37 lines, -12 lines 0 comments Download
M src/cmd/gc/walk.c View 1 2 3 4 5 6 7 8 9 2 chunks +9 lines, -0 lines 0 comments Download
A test/fixedbugs/issue5793.go View 1 2 3 4 5 6 11 1 chunk +36 lines, -0 lines 0 comments Download

Messages

Total messages: 15
cmang
Hello rsc@golang.org, adonovan@google.com (cc: golang-dev@googlegroups.com), I'd like you to review this change to https://code.google.com/p/go
11 years, 6 months ago (2013-09-11 18:45:32 UTC) #1
rsc
please run 'hg gofmt' to reformat the test case (they are not strictly required to ...
11 years, 6 months ago (2013-09-11 18:51:15 UTC) #2
cmang
On 2013/09/11 18:51:15, rsc wrote: > please run 'hg gofmt' to reformat the test case ...
11 years, 6 months ago (2013-09-11 19:32:34 UTC) #3
rsc
oh, right. and i explained why. gofmt -w test/fixedbugs/issue5793.go then. not a big deal. i ...
11 years, 6 months ago (2013-09-11 21:15:27 UTC) #4
rsc
What's here looks good, and we could check it in as an intermediate step if ...
11 years, 6 months ago (2013-09-12 16:15:02 UTC) #5
cmang
It actually does work for the case you mentioned because of the change to inl.c. ...
11 years, 6 months ago (2013-09-12 16:29:06 UTC) #6
rsc
Yes, definitely add a test. I am not sure you can rely on the inlining ...
11 years, 6 months ago (2013-09-12 16:32:46 UTC) #7
cmang
On 2013/09/12 16:32:46, rsc wrote: > Yes, definitely add a test. > > I am ...
11 years, 6 months ago (2013-09-12 17:53:03 UTC) #8
cmang
On 2013/09/12 17:53:03, cmang wrote: > On 2013/09/12 16:32:46, rsc wrote: > > Yes, definitely ...
11 years, 6 months ago (2013-09-13 22:33:46 UTC) #9
rsc
Let's leave this until after Go 1.2. Thanks.
11 years, 5 months ago (2013-09-23 17:34:41 UTC) #10
gobot
Replacing golang-dev with golang-codereviews.
11 years, 2 months ago (2013-12-20 16:26:00 UTC) #11
dave_cheney.net
On 2013/12/20 16:26:00, gobot wrote: > Replacing golang-dev with golang-codereviews. Chris, Could you please bring ...
11 years, 1 month ago (2014-01-31 00:06:22 UTC) #12
cmang
On 2014/01/31 00:06:22, dfc wrote: > On 2013/12/20 16:26:00, gobot wrote: > > Replacing golang-dev ...
11 years, 1 month ago (2014-01-31 02:04:53 UTC) #13
rsc
LGTM https://codereview.appspot.com/13367051/diff/30001/src/cmd/gc/walk.c File src/cmd/gc/walk.c (right): https://codereview.appspot.com/13367051/diff/30001/src/cmd/gc/walk.c#newcode476 src/cmd/gc/walk.c:476: // Use results from call expression as arguments ...
11 years ago (2014-03-05 19:12:15 UTC) #14
rsc
11 years ago (2014-03-05 19:16:25 UTC) #15
*** Submitted as https://code.google.com/p/go/source/detail?r=3e456b047fd9 ***

cmd/gc: allow append and complex builtins to accept 2-result call expression as
first argument.

Fixes issue 5793.

LGTM=rsc
R=rsc, adonovan, dave
CC=golang-codereviews
https://codereview.appspot.com/13367051

Committer: Russ Cox <rsc@golang.org>
Sign in to reply to this message.

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